1097326 - Quote marks occur in EV Locality field when clicking on indicator

Status: REOPENED

(Firefox :: Site Identity, defect, P3)

Description

[him]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:35.0) Gecko/20100101 Firefox/35.0
Build ID: 20141110004002

Steps to reproduce:

Step 1: Visit EV-enabled site which has a Locality field containing non A-Z and 0-9 characters, eg has a comma. Reference site: https://www.positivessl.com/
Step 2: Click EV indicator, observe text under Org name.


Actual results:

The Locality field starts with ", contrary to the certificate contents.


Expected results:

The locality field should not contain characters not in the certificate.

Comment 1

[:Gijs (he/him)]

Johnathan, in my head you're still the person who "did" Larry... do you have any idea why there's a single double quote there, and/or what's meant to happen in this case - and/or who else to ask? :-)

Comment 2

[Dão Gottwald [:dao]]

(In reply to him from comment #0)
> User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:35.0)
> Gecko/20100101 Firefox/35.0
> Build ID: 20141110004002
> 
> Steps to reproduce:
> 
> Step 1: Visit EV-enabled site which has a Locality field containing non A-Z
> and 0-9 characters, eg has a comma. Reference site:
> https://www.positivessl.com/
> Step 2: Click EV indicator, observe text under Org name.
> 
> 
> Actual results:
> 
> The Locality field starts with ", contrary to the certificate contents.

Here's the relevant certificate content according to the Certificate Viewer:

CN = www.positivessl.com
OU = COMODO EV SGC SSL
OU = COMODO EV SSL
O = Comodo CA Ltd
Object Identifier (2 5 4 9) = "3rd Floor,"
Object Identifier (2 5 4 9) = "26 Office Village,"
Object Identifier (2 5 4 9) = "Exchange Quay,  Trafford Road"
L = "Salford,"
ST = Greater Manchester

Comment 3

[Johnathan Nightingale [:johnath]]

As Dao notes - the locality display contains a quote because the Locality field in the certificate *does* contain an opening quote. So that part to me is unsurprising, though weird (an individual building a weird CSR isn't too surprising, but a CA doing it is stranger - and even their own OU/O frields don't have it). I don't know if it's contra-spec to use quotes in this way - there I'd get someone from security to chime in.

What makes this cert particularly bogus-looking for Larry is that the Locality also includes a comma before the closing quote. This exposes a bug, here:

http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser.js#6811

The various subject info fields are all returned as a set of comma-separated fields, which that code then splits on commas. So having a comma within a field name causes us to truncate it as a field boundary. That's our bug even if the cert is screwy (Postel's law!), albeit an edge case.

An easy way to fix it would be to just scrub strings of weird characters before display. A more correct way is probably to do more robust field parsing (and let crazy certs be reflected in Larry as encoded).

Comment 4

[:Gijs (he/him)]

Perfect. Let's backlog this so we can fix it - the description in comment #3 should be ample ammunition to do so.

Permalink to the code referenced in comment #3: https://hg.mozilla.org/mozilla-central/annotate/688f821edcd4/browser/base/content/browser.js#l6811

Comment 5

[BMO Automation]

Per policy at https://wiki.mozilla.org/Bug_Triage/Projects/Bug_Handling/Bug_Husbandry#Inactive_Bugs. If this bug is not an enhancement request or a bug not present in a supported release of Firefox, then it may be reopened.

Comment 6

[:Gijs (he/him)]

The cert for the site in comment #0 has changed (fortunately relevant data is in comment #2), but the code in Firefox has stayed the same (though moved to browser-siteIdentity.js) so I'm pretty sure this is still buggy.

I was reminded of this bug when someone mentioned we now have an X509.jsm .

I'm hoping that means we can swap out some of the bogus parsing with a better version.