Properly EscapeForHTML potentially malicious input from X.509 certificates.

Properly EscapeForHTML potentially malicious input from X.509 certificates.

BUG=142956
TEST=Create an X.509 certificate with a CN field that contains JavaScript.
When you get the SSL error screen, check that the HTML + JavaScript is
escape instead of being treated as HTML and/or script.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=152210

[palmer, 2012-08-16 00:09:33]

thakis as a possible chrome/OWNERS owner. If someone else is better suited, let
me know. Thank you!

[Tom Sepez, 2012-08-16 16:35:33]

LGTM.

[Tom Sepez, 2012-08-16 16:52:21]

Actually, the right place to do this might be in
SSLBlockingPage::GetHTMLContents(). In some sense, the contents of the
SSLErrorInfo are (unsafe) strings independent of the context in which they will
ultimately be presented.

[Tom Sepez, 2012-08-16 16:59:08]

Alternatively, does X.509 put any constraints on these fields which we could
enforce earlier on?

[palmer, 2012-08-16 18:07:11]

> Actually, the right place to do this might be in
> SSLBlockingPage::GetHTMLContents(). In some sense, the contents of the
> SSLErrorInfo are (unsafe) strings independent of the context in which they
will
> ultimately be presented.

Turns out we can't do that, because the unsafe strings are interpolated into
strings generated_resources.grd, which themselves contain markup, and which are
then interpolated into the actual template. So we have to escape here.

[palmer, 2012-08-16 18:23:12]

> Alternatively, does X.509 put any constraints on these fields which we could
> enforce earlier on?

I don't know if we can; in any case, we'd still want escaping code like this for
defense in depth. Also in any case, such jibber-jabber should not get
interpreted as a valid CN for the purposes of certificate validation, and I
can't get XSS bits to get executed in other X.509 fields in other Chrome
contexts too (such as the certificate importer, the certificate viewer, and the
info bubble).

[palmer, 2012-08-17 17:45:31]

Adding brettw for OWNERS happiness. Thanks!

[Nico, 2012-08-17 20:50:09]

tsepez: Are you happy with this?

[Tom Sepez, 2012-08-17 20:52:49]

LGTM.

[Nico, 2012-08-17 20:58:44]

lgtm stamp

You might want to add a chrome/browser/ssl/OWNERS file.

[commit-bot: I haz the power, 2012-08-17 21:01:56]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/palmer@chromium.org/10827364/1

[commit-bot: I haz the power, 2012-08-18 01:58:47]

Change committed as 152210