Closed
Bug 666516
Opened 11 years ago
Closed 11 years ago
overriding "@mozilla.org/security/certoverride;1" causes a crash in the certManager.xul
Categories
(Core :: Security: PSM, defect, P1)
Core
Security: PSM
Tracking
()
RESOLVED
FIXED
mozilla9
Tracking | Status | |
---|---|---|
firefox8 | --- | fixed |
People
(Reporter: passfree, Assigned: bzbarsky)
Details
(Whiteboard: [qa?])
Attachments
(1 file)
5.52 KB,
patch
|
KaiE
:
review+
blizzard
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_6_7) AppleWebKit/535.1 (KHTML, like Gecko) Chrome/14.0.797.0 Safari/535.1 Build Identifier: xulrunner (all versions) This is an old bug and it was reported like 2 years ago. I thought it was fixed but that is not the case in xul2.0 and perhaps even xul5.0 In summary, if we override @mozilla.org/security/certoverride;1 via the following chrome manifest component {6c367300-5a77-11df-a08a-0800200c9a61} components/CertOverrideService.js contract @mozilla.org/security/certoverride;1 {6c367300-5a77-11df-a08a-0800200c9a61} A crash will occur within certManager.xul when the following line is invoked serverTreeView.loadCertsFromCache(certcache, nsIX509Cert.SERVER_CERT); As far as I remember the issue was somehow related to the way the XPCOM instance was handled in the C code (old code conventions?). This service @mozilla.org/security/certoverride;1 is actually overridden by several popular extensions and products such as Selenium. A legitimate reason for overriding it is to hide bad ssl certificate problems. Reproducible: Always Steps to Reproduce: See above. Actual Results: The app crashes when certManager.xul is opened. Expected Results: It should not crash.
Comment 1•11 years ago
|
||
You mean bug 564584 which is marked fixed
Component: XPConnect → Security: UI
QA Contact: xpconnect → ui
![]() |
Assignee | |
Comment 3•11 years ago
|
||
If you're getting a crash here, what's the stack look like? Is there a breakpad incident id?
![]() |
Assignee | |
Comment 4•11 years ago
|
||
Actually, nevermind. nsCertTree::GetCertsByTypeFromCertList has exactly the sort of code that was causing crashes in bug 564584, and the thing it's working with was gotten by contract id... <sigh>.
Severity: major → critical
Status: UNCONFIRMED → NEW
Ever confirmed: true
![]() |
Assignee | |
Updated•11 years ago
|
Component: Security: UI → Security: PSM
QA Contact: ui → psm
![]() |
Assignee | |
Comment 6•11 years ago
|
||
This is ridiculous. This code should never have been written....
Attachment #555170 -
Flags: review?(kaie)
![]() |
Assignee | |
Updated•11 years ago
|
Assignee: nobody → bzbarsky
Priority: -- → P1
Comment 7•11 years ago
|
||
Comment on attachment 555170 [details] [diff] [review] Fix Yes, I agree this code wasn't written as future-proof as it should had. And yes, we need more people caring for PSM and helping out with it, in particular with writing/reviewing patches and fixing correctness issues. Thanks a lot for your fix, I appreciate it. r=kaie
Attachment #555170 -
Flags: review?(kaie) → review+
![]() |
Assignee | |
Updated•11 years ago
|
Whiteboard: [need landing]
![]() |
Assignee | |
Comment 8•11 years ago
|
||
http://hg.mozilla.org/integration/mozilla-inbound/rev/9c84cf6b4d2c
Flags: in-testsuite?
Whiteboard: [need landing]
Target Milestone: --- → mozilla9
![]() |
Assignee | |
Comment 9•11 years ago
|
||
Comment on attachment 555170 [details] [diff] [review] Fix Given that this is causing extension crashes, we should at least consider it for aurora.
Attachment #555170 -
Flags: approval-mozilla-aurora?
Comment 10•11 years ago
|
||
Comment on attachment 555170 [details] [diff] [review] Fix Approved, please land on Aurora (Firefox Update 8.)
Attachment #555170 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 11•11 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/9c84cf6b4d2c
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
![]() |
Assignee | |
Comment 12•11 years ago
|
||
http://hg.mozilla.org/releases/mozilla-aurora/rev/9546f9e05e7a
status-firefox8:
--- → fixed
Comment 13•11 years ago
|
||
Can QA get some help verifying this bug fix? Either passfree, if you can verify this on Firefox 8 Aurora, or if you can provide some more details on how to use the testcase you provided. It's not completely clear to QA how to utilize your testcase. Thanks
Whiteboard: [qa?]
Reporter | ||
Comment 14•11 years ago
|
||
I have verified that it works in 8. Though, I am not completely sure if this bug has been fully fixed everywhere.
You need to log in
before you can comment on or make changes to this bug.
Description
•