1031984 - Bad sanitization of SSL certificate's "common name"

Status: RESOLVED DUPLICATE of bug 1415279

(Core :: Security: PSM, defect, P3)

Description

[Hugues Alary]

Attachment: ff30SSLBug.png

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:30.0) Gecko/20100101 Firefox/30.0 (Beta/Release)
Build ID: 20140605174243

Steps to reproduce:

Create an SSL certificate and input     ">   (quote followed by a closing tag)  as the "common name":

openssl req -new -x509 -days 365 -nodes -out ./server.pem -keyout server.key
Common Name (e.g. server FQDN or YOUR name) []:">

Now, use that certificate to serve an HTTPS page. The server hostname isn't important.


Actual results:

1- The SSL connection being untrusted, the browsers loads the following page:

about:certerror?e=nssBadCert&u=https%3A//example.com&c=UTF-8&f=regular&d=example.com%20uses%20an%20invalid%20security%20certificate.%0A%0AThe%20certificate%20is%20not%20trusted%20because%20it%20is%20self-signed.%0AThe%20certificate%20is%20only%20valid%20for%20<a%20id%3D"cert_domain_link"%20title%3D"\">%20">\">%20</a>%0A%0A%28Error%20code%3A%20sec_error_untrusted_issuer%29%0A

2- And shows the following error message:
example.com uses an invalid security certificate. The certificate is not trusted because it is self-signed. 
The certificate is only valid for \ 
(Error code: sec_error_untrusted_issuer)

3- Note the "The certificate is only valid for \". That's the problem.




Expected results:

The message should say:

The certificate is only valid for ">

It appears to me that the problem stems in the way the Common-Name is escaped:

FF uses the Common-Name to create an <a></a> tag as such:
<a id="cert_domain_link" title="common-name">common-name</a>

In the case of our malformed Common-Name the <a> tag ends looking like this:
<a id="cert_domain_link" title="\">">\"></a>


I spent a few hours trying to exploit this bug, in the browser. I haven't been able to so far; and reading the source code makes me think that it might not be possible to exploit it, *in the browser*.

However, using a Common-Name such as:
" onclick=javascript:alert('hello')>ClickHere</a><a>

Would redirect to this link:
about:certerror?e=nssBadCert&u=https%3A//example.com&c=UTF-8&f=regular&d=example.com%20uses%20an%20invalid%20security%20certificate.%0A%0AThe%20certificate%20is%20not%20trusted%20because%20it%20is%20self-signed.%0AThe%20certificate%20is%20only%20valid%20for%20%3Ca%20id%3D%22cert_domain_link%22%20title%3D%22%5C%22%20onclick%3Djavascript%3Aalert%28%27hello%27%29%3EClickHere%3C/a%3E%3Ca%3E%22%3E%5C%22%20onclick%3Djavascript%3Aalert%28%27hello%27%29%3EClickHere%3C/a%3E%3Ca%3E%3C/a%3E%0A%0A%28Error%20code%3A%20sec_error_untrusted_issuer%29%0A

...the &d parameter would look like this decoded:
example.com uses an invalid security certificate.

The certificate is not trusted because it is self-signed.
The certificate is only valid for <a id="cert_domain_link" title="\" onclick=javascript:alert('hello')>ClickHere</a><a>">\" onclick=javascript:alert('hello')>ClickHere</a><a></a>

(Error code: sec_error_untrusted_issuer)

...and anybody trusting this a bit too much could end up executing the alert('hello')

Comment 1

[Hugues Alary]

You can use this certificate to reproduce the bug

-----BEGIN PRIVATE KEY-----
MIIEvwIBADANBgkqhkiG9w0BAQEFAASCBKkwggSlAgEAAoIBAQDWW9Hi1lW3E59B
dkkDdMld9e/yY7ZoQ7Qszzmrv/nzp68hbyCkEK8qxQPAfvvcATmR4FINN9UFy9Fw
aelavkoVWPMJi0j+Chch7zpxSKdajylB9m8CHVSxDqHL5N7bLvrrvdssAd5qe/IK
aT9sqNhvyg5fI04tuXcBtxPnLemuifp5+TM5QfuA9ywC01c0WmmqET5Wb65cf7vy
p/G0EdSeVz0EgmnVDEsOGgO1ZTDIP1mZUVCz2qRtUf4eUM1BIaAGIHBb4hIeTGnL
gZRraRLc9gzWekEYK7jTQTNqSLwm0oeyIOI2NLt1x5Ex0PRCo8bBbrxUjBHnLjHI
SGEQYcmdAgMBAAECggEAc04+74sl3m4Sq47qJY7LxtxFbxhOBXXt1HC/7/A7juJz
ZUO2xionX5VMuF5WMnkCM8L7wBp5Xz3M8nA2U1aq2SLY0Rh4chaRbPKXeFqBk0gR
BGeAPR3z8a4SQHBjpHl4BhlBbUGwF42GnC0TLesBSdjhtDF0m0n5hxWpAiEDY5ve
FXGerNlMBCSvdbfHUHOP1VXqjQTVlf5H0LaecqBkoFcM5jFGK+8WFLZBzobWXjM1
/msS8rp7gq7f9oOb7yoHWlDTY6ZRwFl2pHsZ9VacleFw5ljid8IWz24KxI/tyLcE
QYq4mUJT72vAFpqRQewByfSdttva8o5LP/FjJE0WIQKBgQD+u9YFsehTFGv1Exd7
i2Dmet6tPgmJBOeAG5dbUlTjBAOpgFpJb3+eIssk8jkmw0JOoZ27MUxte1vB8Frx
9Z/9A466SBSQdM9t3L/BUemTERLz3KSDYoPsL0J7TtUCAq43KEWVMA6/4mwkZaK/
2aU4i1kMnkQSGw3rtvThLRUFiQKBgQDXbJqpm4nIZPyHzuE2ZP2bJErJIpHqYooG
GHnS/KexpJUcuAMPSVBUcWUHhj+H0pDMEZFIroAymBoEOX7GRDVkNSG6sUFM1b1y
dcUvAkkDt8WQwOO2zmRxLos4Cobq4opOhALVQz4MAOzjuK2E/Z6bh5hsPxxv5uYH
PKcVCkWydQKBgQCX8Qx4+yRbodu/I/Mm0v0qnPGDnnRrkUxQoqSGaTaO7o8MpK22
DdauSYyobf7e+yrq4gXGaWJwD1us86QbnOogLeYNgP8bJ0GvAzQxqJ1NBqsqYFqU
r8eGsq3SBTSS7WxjJBdVJIQ3umPM1f7ctoKRlrS4DRMHRbHo/olUYuKeGQKBgQCi
0JZkMgYEyy6Bhj542gNCyj8rnVEjEK64xK7uY75qaLW6KulFGGf6KmntMe26Pc7q
LSzc/eYuVv7yynGib9Lalb2UWHu+Ep1IcPJDATsl8DMFoIN6mqsXT9Iw3OPzipx1
kxiBDn1KnThtNBsRNI43hlgRF/HqMV0RgynUoRrkMQKBgQCseVQPhM0laqlBw0HS
wAnAdfIQiyi0kAYkG+yfmmVTeBURyIqdAOK7eD+nXQrl0otBWfMezH/By6b36R7G
YwEE4+HuxlwNAcWmc7gZx0jlXHRuhzTe4rJYRvqQ211englhJojB+z6ax2l/Qbuh
4JXWiHzMZmRGy8cxOrbi2Fhyhw==
-----END PRIVATE KEY-----
-----BEGIN CERTIFICATE-----
MIIDdzCCAl+gAwIBAgIJAPhnDyTHCCgMMA0GCSqGSIb3DQEBBQUAMFIxCzAJBgNV
BAYTAkFVMRMwEQYDVQQIDApTb21lLVN0YXRlMSEwHwYDVQQKDBhJbnRlcm5ldCBX
aWRnaXRzIFB0eSBMdGQxCzAJBgNVBAMMAiI+MB4XDTE0MDYzMDAzMjAzNloXDTE1
MDYzMDAzMjAzNlowUjELMAkGA1UEBhMCQVUxEzARBgNVBAgMClNvbWUtU3RhdGUx
ITAfBgNVBAoMGEludGVybmV0IFdpZGdpdHMgUHR5IEx0ZDELMAkGA1UEAwwCIj4w
ggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQDWW9Hi1lW3E59BdkkDdMld
9e/yY7ZoQ7Qszzmrv/nzp68hbyCkEK8qxQPAfvvcATmR4FINN9UFy9FwaelavkoV
WPMJi0j+Chch7zpxSKdajylB9m8CHVSxDqHL5N7bLvrrvdssAd5qe/IKaT9sqNhv
yg5fI04tuXcBtxPnLemuifp5+TM5QfuA9ywC01c0WmmqET5Wb65cf7vyp/G0EdSe
Vz0EgmnVDEsOGgO1ZTDIP1mZUVCz2qRtUf4eUM1BIaAGIHBb4hIeTGnLgZRraRLc
9gzWekEYK7jTQTNqSLwm0oeyIOI2NLt1x5Ex0PRCo8bBbrxUjBHnLjHISGEQYcmd
AgMBAAGjUDBOMB0GA1UdDgQWBBR+ETTzwoNfip6C9RmIgx9Hw+KuBjAfBgNVHSME
GDAWgBR+ETTzwoNfip6C9RmIgx9Hw+KuBjAMBgNVHRMEBTADAQH/MA0GCSqGSIb3
DQEBBQUAA4IBAQCNgOyh0r9HZHcpD0PPuBWJA+6kmBq2PRRLlxjYyfQXQ26YQbI0
QP1R4opWqVNdXpeUKr5PfarwgStwP0vO7iZYvW7yV7J5a7we/yFGvr8mnVG9tBit
Fp/Q20AwTvHwAxaLUUAO+876EL1jO/YZqkgiFTxklBy04ayJ4wAb50RQKQkdZPZ3
JKu9f9oHzo6kdE78k3yS9/ifkk6l+UfcfOP1Ap1e57XlEnGv+IEG83+uqyhxGc+n
6W+093lIRniohYWW8fODD5PLi+wmYbC0C8BdRvjDGPVExZfTnpFcDNyUTC93/Raa
sF5fzKdWO2mjYRbtvpRk+meB3TZpGZ1O62jj
-----END CERTIFICATE-----

Comment 2

[Hugues Alary]

Attachment: server-certificate

Comment 3

[Hugues Alary]

You can connect to this server with your browser to experience the bug: https://docker.betabrand.io/

Comment 5

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

I'm not an expert in HTML sanitization, but I think the code already protects against this: http://hg.mozilla.org/mozilla-central/file/4a9353b5762d/browser/base/content/aboutcerterror/aboutCertError.xhtml#l107
Yes, the display may look ugly, but I think it will only ever be rendered as text and/or a link that is "safe" to click (i.e. no surprise onclick=javascript... attributes).

Comment 6

[Hugues Alary]

The javascript code does indeed protect against this. However, this is just plain luck. The dev who wrote the code, luckily used createTextNode() instead of, let's say, innerhtml().

Ultimately, the function handling the sanitization and returning the sanitized string to the javascript is returning a wrongly sanitized string.

Someone else, tomorrow, could take this badly sanitized string, write some javascript that takes it in input and ... create an XSS vulnerability.

Comment 7

[Daniel Veditz [:dveditz]]

(In reply to Hugues Alary from comment #6)
> The javascript code does indeed protect against this. However, this is just
> plain luck. The dev who wrote the code, luckily used createTextNode()
> instead of, let's say, innerhtml().

Not luck, this is required. innerHTML is evil and stomped out whenever we see it, and we must ALWAYS use things like createTextNode() or node.textContent.

Comment 8

[Daniel Veditz [:dveditz]]

CCing johnath for giggles.

Comment 9

[Hugues Alary]

(In reply to Daniel Veditz [:dveditz] from comment #7)
> Not luck, this is required. innerHTML is evil and stomped out whenever we
> see it, and we must ALWAYS use things like createTextNode() or
> node.textContent.

> CCing johnath for giggles.

Cool. I don't know anything about mozilla coding practices and it's the first time I even ever report a bug here, so pardon my ignorance and sorry if I'm wasting someone's time.

Comment 10

[Johnathan Nightingale [:johnath]]

:) Yeah, I used createTextNode on purpose, but this is still a sad day for what remains of my code in tree. I am not as ashamed as I could be with an innerHtml or if I had left the page privileged, but I am still ashamed. 

@Hugues - no apology necessary at all - thank you for finding this and reporting it! Are you interested in trying to write a patch for it?

Comment 11

[Hugues Alary]

Johnathan, I'd be happy to try patching it, I however have no clues as to where I'd start.

I did search from the source code for the function responsible for sanitizing the CN, to no avail.

I read the code in http://hg.mozilla.org/mozilla-central/file/4a9353b5762d/browser/base/content/aboutcerterror/aboutCertError.xhtml#l107 but, unless I'm mistaken, I believe the real problem lies a level before that code.

My gut (and my text editor) tells me that maybe the code I'm looking for is laying around here: http://hg.mozilla.org/mozilla-central/file/4a9353b5762d/security/nss/lib/certdb/alg1485.c

So... it might be faster for someone else to patch this ;)

Comment 12

[Hugues Alary]

After digging a bit more into this, I realize that the code sanitizing the string is perfectly correct.

The problem then lies in the JS. 

However, it would seem weird to me that the JS be in charge of making sure the CN that is passed to him doesn't contained \". As a programmer, if the CN is    ">     I'd expect to receive    ">     not     \">


For a given common name:   ">

Should the JS expect an RFC 1779 escaped string (  \">   ), or, should the JS expect a "fully usable" string (  ">   )? I guess the patch depends a bit on the answer to this question.

Comment 13

[Johnathan Nightingale [:johnath]]

The proper expectation from NSS is a bit beyond my ken, hopefully Brian or Kai, cc'd, have a better answer. For the purposes of this bug, we could just assert that defense in depth is valuable and that we trust no one else to sanitize site-supplied content, even NSS.

Comment 14

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

This specific problem could be solved by having TransportSecurityInfo properly escape strings before using them to build some html it passes back to the docshell: http://hg.mozilla.org/mozilla-central/annotate/2d88803a0b9c/security/manager/ssl/src/TransportSecurityInfo.cpp#l750
More generally, however, there's probably a safer way to load error pages than pass around strings with html from untrusted sources.

Comment 15

[Hugues Alary]

> More generally, however, there's probably a safer way to load error pages than pass around strings with html from untrusted sources.

I'd tend to agree with this.

Not sure if somebody's waiting on me for that one, but if so, the current state of the discussion (and my absolute 0 knowledge of FF codebase combined to years not touching CPP) doesn't make me feel comfortable in trying to patch the bug, as I feel I might patch it the wrong way.

Comment 16

[Al Billings [:abillings - ex-MoCo]]

The bounty triage committee has determined this bug to not qualify for bounty given the nature of the issue and its impact.

Comment 17

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

This was partially addressed by bug 1261936 (although perhaps something similar can be done with an entry in the SAN?). In any case, the real solution here is to not have PSM try and generate HTML itself, and to rely on the front-end for things like that.

Comment 18

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

Bug 1415279 removed the code that I was concerned about.