Closed Bug 1165911 Opened 5 years ago Closed 5 years ago

GatherEKUTelemetry fails when parsing certs with no X.509v3 extensions at all


(Core :: Security: PSM, defect)

36 Branch
Not set



Tracking Status
firefox38 --- wontfix
firefox38.0.5 --- wontfix
firefox39 --- fixed
firefox40 --- fixed
firefox41 --- fixed
firefox-esr31 --- unaffected
firefox-esr38 --- fixed
b2g-v2.0 --- unaffected
b2g-v2.0M --- unaffected
b2g-v2.1 --- unaffected
b2g-v2.1S --- unaffected
b2g-v2.2 --- fixed
b2g-master --- fixed


(Reporter: dkg, Assigned: keeler)




(3 files, 1 obsolete file)

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:37.0) Gecko/20100101 Firefox/37.0 Iceweasel/37.0.2
Build ID: 20150420231403

Steps to reproduce:

with iceweasel 38.0-2 (from debian), visit in safe-mode.

(this is noted on

The segmentation fault appears to be from a dereference of endEntityCert->extensions when no extensions are present in the X.509v3 certificate (which is the case for at least one of the certs offered by

I'm attaching the certificates currently offered from, as recorded via "gnutls-cli --print-cert"

I ran into the same problem with 37.0.2, fwiw.

Actual results:


Expected results:

no segfault :)
David, does it ring a bell?
FWIW, I don't have a crash with 38.0.5 b2.
Ever confirmed: true
Flags: needinfo?(dkeeler)
Attached patch patch (obsolete) — Splinter Review
Ah, yep - missed a null check. I added some extra safety checks as well. Eventually we shouldn't be relying on NSS to extract these values - we should use mozilla::pkix.
Assignee: nobody → dkeeler
Flags: needinfo?(dkeeler)
Attachment #8607137 - Flags: review?(cykesiopka.bmo)
Comment on attachment 8607137 [details] [diff] [review]

Review of attachment 8607137 [details] [diff] [review]:

LGTM, but is this (should it be) tested anywhere?

::: security/manager/ssl/src/SSLServerCertVerification.cpp
@@ +911,3 @@
>    if (rv != SECSuccess || !isBuiltIn) {
> +           ("BR telemetry: issuer of '%s' is not a built-in root (or "

Nit: maybe "root" or "root cert" instead of "issuer"?
Attachment #8607137 - Flags: review?(cykesiopka.bmo) → review+
Component: Untriaged → Security: PSM
OS: Unspecified → All
Product: Firefox → Core
Hardware: Unspecified → All
Version: 38 Branch → 36 Branch
Duplicate of this bug: 1166129
Thanks for the review. Testing this would require something like obtaining a certificate issued by a built-in CA with no extensions (which shouldn't exit, according to the BRs). I think we could probably manage it, but the benefit to effort ratio would be small.
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
Attached patch patch as landedSplinter Review
Approval Request Comment
[Feature/regressing bug #]: bug 968817
[User impact if declined]: potential null-dereference crashes on systems with custom built-in root certificates
[Describe test coverage new/current, TreeHerder]: manually tested
[Risks and why]: low - this just does some additional null checks
[String/UUID change made/needed]: none
Attachment #8607137 - Attachment is obsolete: true
Attachment #8608368 - Flags: review+
Attachment #8608368 - Flags: approval-mozilla-aurora?
[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration:
User impact if declined: potential null-dereference crashes on systems with custom built-in root certificates
Fix Landed on Version: 41
Risk to taking this patch (and alternatives if risky): low - this just does some additional null checks
String or UUID changes made by this patch: none

See for more info.
Attachment #8608369 - Flags: review+
Attachment #8608369 - Flags: approval-mozilla-esr38?
Attachment #8608369 - Flags: approval-mozilla-beta?
Comment on attachment 8608369 [details] [diff] [review]
patch for beta, esr-38

This also applies to b2g37 (v2.2)

NOTE: Please see to better understand the B2G approval process and landings.

[Approval Request Comment]
see previous comments
Attachment #8608369 - Flags: approval-mozilla-b2g37?
Comment on attachment 8608368 [details] [diff] [review]
patch as landed

Approved for uplift to aurora to improve stability
Attachment #8608368 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment on attachment 8608369 [details] [diff] [review]
patch for beta, esr-38

Approved for uplift to beta, should improve stability
Attachment #8608369 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment on attachment 8608369 [details] [diff] [review]
patch for beta, esr-38

Also taking this for esr 38.
Attachment #8608369 - Flags: approval-mozilla-esr38? → approval-mozilla-esr38+
Comment on attachment 8608369 [details] [diff] [review]
patch for beta, esr-38

Approved for uplift to b2g37 to improve stability.
Please NI if causing any issue.
Attachment #8608369 - Flags: approval-mozilla-b2g37? → approval-mozilla-b2g37+
You need to log in before you can comment on or make changes to this bug.