225034 - Certificate Manager Crashes Mozilla [@ nsCertTree::CmpByCrit]

Status: VERIFIED FIXED

(Core Graveyard :: Security: UI, defect)

Description

[David E. Ross]

User-Agent:       Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.5) Gecko/20031007
Build Identifier: Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.5) Gecko/20031007

I want to view my certificates.  When I attempt to open Certificate Manager,
however, Mozilla crashes.  

Reproducible: Always

Steps to Reproduce:
1.  Select Edit > Preferences.
2.  On the Preferences window, select Privacy & Security > Certificates.  
3.  On the Certificates window, select Manage Certificates. 

Actual Results:  
Mozilla crashed.  

Expected Results:  
I should be able to view my certificates.  Certificates from CAs should be
editable regarding trust.  

Yes, I sent a TalkBack message.

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

->PSM

Comment 2

[Simon Montagu :smontagu]

Stack from Talkback:

nsCertTree::CmpByCrit
[c:/builds/seamonkey/mozilla/security/manager/ssl/src/nsCertTree.cpp, line 979]
nsCertTree::CmpBy
[c:/builds/seamonkey/mozilla/security/manager/ssl/src/nsCertTree.cpp, line 1013]
nsCertTree::CmpCACert
[c:/builds/seamonkey/mozilla/security/manager/ssl/src/nsCertTree.cpp, line 1034]
nsCertTree::GetCertsByTypeFromCertList
[c:/builds/seamonkey/mozilla/security/manager/ssl/src/nsCertTree.cpp, line 307]
nsCertTree::GetCertsByTypeFromCache
[c:/builds/seamonkey/mozilla/security/manager/ssl/src/nsCertTree.cpp, line 346]
nsCertTree::LoadCertsFromCache
[c:/builds/seamonkey/mozilla/security/manager/ssl/src/nsCertTree.cpp, line 366]
XPTC_InvokeByIndex
[c:/builds/seamonkey/mozilla/xpcom/reflect/xptcall/src/md/win32/xptcinvoke.cpp,
line 102]
XPCWrappedNative::CallMethod
[c:/builds/seamonkey/mozilla/js/src/xpconnect/src/xpcwrappednative.cpp, line 2019]
XPC_WN_CallMethod
[c:/builds/seamonkey/mozilla/js/src/xpconnect/src/xpcwrappednativejsops.cpp,
line 1270]
js_Invoke [c:/builds/seamonkey/mozilla/js/src/jsinterp.c, line 845]
js_Interpret [c:/builds/seamonkey/mozilla/js/src/jsinterp.c, line 2861]
js_Invoke [c:/builds/seamonkey/mozilla/js/src/jsinterp.c, line 861]
js_InternalInvoke [c:/builds/seamonkey/mozilla/js/src/jsinterp.c, line 936]
JS_CallFunctionValue [c:/builds/seamonkey/mozilla/js/src/jsapi.c, line 3540]
nsJSContext::CallEventHandler
[c:/builds/seamonkey/mozilla/dom/src/base/nsJSEnvironment.cpp, line 1220]
nsJSEventListener::HandleEvent
[c:/builds/seamonkey/mozilla/dom/src/events/nsJSEventListener.cpp, line 183]
nsEventListenerManager::HandleEventSubType
[c:/builds/seamonkey/mozilla/content/events/src/nsEventListenerManager.cpp, line
1195]
nsEventListenerManager::HandleEvent
[c:/builds/seamonkey/mozilla/content/events/src/nsEventListenerManager.cpp, line
1879]
GlobalWindowImpl::HandleDOMEvent
[c:/builds/seamonkey/mozilla/dom/src/base/nsGlobalWindow.cpp, line 852]
DocumentViewerImpl::LoadComplete
[c:/builds/seamonkey/mozilla/content/base/src/nsDocumentViewer.cpp, line 952]
nsDocShell::EndPageLoad
[c:/builds/seamonkey/mozilla/docshell/base/nsDocShell.cpp, line 4333]
nsWebShell::EndPageLoad
[c:/builds/seamonkey/mozilla/docshell/base/nsWebShell.cpp, line 800]
nsDocShell::OnStateChange
[c:/builds/seamonkey/mozilla/docshell/base/nsDocShell.cpp, line 4267]
nsDocLoaderImpl::FireOnStateChange
[c:/builds/seamonkey/mozilla/uriloader/base/nsDocLoader.cpp, line 1215]
nsDocLoaderImpl::doStopDocumentLoad
[c:/builds/seamonkey/mozilla/uriloader/base/nsDocLoader.cpp, line 870]
nsDocLoaderImpl::DocLoaderIsEmpty
[c:/builds/seamonkey/mozilla/uriloader/base/nsDocLoader.cpp, line 768]
nsDocLoaderImpl::OnStopRequest
[c:/builds/seamonkey/mozilla/uriloader/base/nsDocLoader.cpp, line 698]
nsLoadGroup::RemoveRequest
[c:/builds/seamonkey/mozilla/netwerk/base/src/nsLoadGroup.cpp, line 704]
imgRequestProxy::RemoveFromLoadGroup
[c:/builds/seamonkey/mozilla/modules/libpr0n/src/imgRequestProxy.cpp, line 161]
imgRequestProxy::OnStopRequest
[c:/builds/seamonkey/mozilla/modules/libpr0n/src/imgRequestProxy.cpp, line 422]
imgRequest::OnStopRequest
[c:/builds/seamonkey/mozilla/modules/libpr0n/src/imgRequest.cpp, line 693]
ProxyListener::OnStopRequest
[c:/builds/seamonkey/mozilla/modules/libpr0n/src/imgLoader.cpp, line 858]
imgCacheValidator::OnStopRequest
[c:/builds/seamonkey/mozilla/modules/libpr0n/src/imgLoader.cpp, line 998]
nsJARChannel::OnStopRequest
[c:/builds/seamonkey/mozilla/netwerk/protocol/jar/src/nsJARChannel.cpp, line 673]
nsCOMPtr_base::assign_with_AddRef
[c:/builds/seamonkey/mozilla/xpcom/glue/nsCOMPtr.cpp, line 71]
nsInputStreamPump::OnStateStop
[c:/builds/seamonkey/mozilla/netwerk/base/src/nsInputStreamPump.cpp, line 484]
nsInputStreamPump::OnInputStreamReady
[c:/builds/seamonkey/mozilla/netwerk/base/src/nsInputStreamPump.cpp, line 325]

Comment 3

[Stephane Saux]

Please see bug 224200: "[e-mail] shouldn't be the default owner for any
mozilla component". I wish I had time to manage these problems, but I don't.
These bugs are important, but assigning them to me will not get them solved,
which would be a shame.

Comment 4

[Brendan Eich [:brendan]]

Who should own this bug?  Default assignees usually propose their successors,
and it would help to know if you have someone in mind.

Simon, can you get the talkback register data, and instructions around the bad
pc (eip, whatever)?  Is this a null cert pointer dereference?

/be

Comment 5

[timeless]

Attachment: the second arg seems to be null (qi failure)

Comment 6

[Brendan Eich [:brendan]]

Comment on attachment 136010 [details] [diff] [review]
the second arg seems to be null (qi failure)

Fine wallpaper, but why is the element not QI'ing to nsIX509Cert?  What else
could it be?

/be

Comment 7

[Simon Montagu :smontagu]

Registers:
EAX: 02b3d7f8 EBX: 02b2746c ECX: 029ef570 EDX: 0000007d
ESI: 00000000 EDI: 029ef570 ESP: 0065ea74 EBP: 0065ea80
EIP: 6184dc46 cf PF af zf sf of IF df nt RF vm   IOPL: 0
CS: 0197 DS: 019f SS: 019f ES: 019f FS: 69d7 GS: 0000

Code Around the PC:
6184dc46 807c330800       cmp     byte ptr [ebx+esi+0x8],0x0
6184dc4b 7510             jnz     6184dc5d
6184dc4d 56               push    esi
6184dc4e ff7518           push    dword ptr [ebp+0x18]
6184dc51 53               push    ebx
6184dc52 ff7508           push    dword ptr [ebp+0x8]
6184dc55 e8b1feffff       call    6184db0b
6184dc5a 83c410           add     esp,0x10
6184dc5d 8b7d14           mov     edi,[ebp+0x14]
6184dc60 807c370800       cmp     byte ptr [edi+esi+0x8],0x0
6184dc65 7541             jnz     6184dca8

Comment 8

[Stephane Saux]

In bug 224200, I proposed Kai Engert as the new default owner. Kai aquiesced.

Comment 9

[Brendan Eich [:brendan]]

Ok, over to Kai; I hope he has time to help. I'm not comfortable wallpapering
over some deeper bug with a null-check, unless it fixes all symptoms -- in which
case it's a good short-term patch for 1.6b.

/be

Comment 10

[Kai Engert (:KaiE:)]

Using yesterday's sources, I can not reproduce this on Linux.

I tried my own debug builds of Seamonkey, Firebird and Thunderbird, with empty
and filled cert database. None crashed (I see some assertion failures in
nsGREDirServiceProvider, though).

I can not reproduce on Linux using a 2003-11-22 trunk build from mozilla.org

I'm downloading a Windows build now.

Comment 11

[Kai Engert (:KaiE:)]

Although I'm not yet able to reproduce, let me think out loud about the code:

This code constructs a new empty certarray instance and fills it.

The only tries to access objects it has added to the array itself, "i < count".

Whenever count gets increased, the code has either inserted or appended a new
object.

Only objects of type nsNSSCertificate are inserted, a type which implements
nsIX509Cert.

I conclude, if the object we obtained using ElementAt from the nsISupportsArray
is not QIable to nsIX509Cert, the implementation of the array must be broken, or
memory in general be corrupted.

Comment 12

[Kai Engert (:KaiE:)]

Comment on attachment 136010 [details] [diff] [review]
the second arg seems to be null (qi failure)

As long as we don't understand the cause, I disagree with the patch. IMHO, if
the array code were working, we would never obtain null.

Comment 13

[Kai Engert (:KaiE:)]

I can not reproduce the crash on Windows 2000 using a 2003-11-22 trunk build.

David, do you still see the bug?

Is anybody else able to reproduce?

Comment 14

[David E. Ross]

I am still using the Mozilla 1.5 version against which this bug was originally
reported.  I normally install only release candidates and operational versions.
I have to download through a dial-up modem and prefer not to spend time with
less stable versions.  Thus, I am not using any attempted corrections.  

I just did an experiment.  With my cache purged and no connection to the
Internet, Mozilla crashes when I try to open Certificate Manager.   With a small
cache and a prior successful access to a secure Web site (thus using a
certificate), Mozilla does not crash when I try to open Certificate Manager.  

If you wish, I will experiment further to refine the conditions under which I
crash.  If necessary (because no one else can reproduce the problem), I will
also install a test version (but only because I consider this a critical bug). 
(Contact me directly for that rather than propagating additional comments.)

Comment 15

[Christian :Biesinger (don't email me, ping me on IRC)]

           nsCOMPtr<nsISupports> isupport = 
             dont_AddRef(certarray->ElementAt(i));

maybe there is no element at position i, so ElementAt returns null, hence qi
also returns null?

Comment 16

[Kai Engert (:KaiE:)]

As I understand the code, "i" is always from zero to [count-1], and count is the
number of elements this code has inserted/appened to the array.

I think the element at "i" can only be null if either the array code is broken,
or new is unable to allocate memory.


With the additional information David gave, I will do some more experiments this
evening. I'll also try whether I can reproduce using the version David uses.

Comment 17

[Asa Dotzler [:asa]]

Kai, have you had a chance to look into this any further? Can we take the patch
for 1.6 and leave the bug open for further work.

Comment 18

[Asa Dotzler [:asa]]

I'm unable to reproduce this with a clean profile with an empty cert datbase on
winXP.  How common do we think is this crash? If it's not highly visible and
we're not near a solution then I don't think it should block the beta. 

Comment 19

[Kai Engert (:KaiE:)]

I'm still not able to reproduce the crash.


After reading the patch in more detail, I think it changes the current logic.
If we were interested in adding code, that protects us from passing in NULL to
the compare function, but keep the current logic, we could write:

-          if ((*aCertCmpFn)(aCertCmpFnArg, pipCert, cert) < 0) {
+          if (cert && (*aCertCmpFn)(aCertCmpFnArg, pipCert, cert) < 0) {

But still, I do not see how "cert" could ever be NULL in this loop.


I ran an additional test with a compiled Firebird build on Linux.
When opening cert manager I see the following assertions. Could those somehow
indicate an broken internal memory situation that possibly triggers the other
crash? Just a wild guess, though.
###!!! ASSERTION: URI mapped to two different specs?: 'uriMapEntry->mDocMapEntry
== nsnull', file /home/kai/mozb/mozilla/xpcom/io/nsFastLoadFile.cpp, line 422
Break: at file /home/kai/mozb/mozilla/xpcom/io/nsFastLoadFile.cpp, line 422
WARNING: chrome: failed to get base url for chrome://help/content/contextHelp.js
-- using wacky default, file
/home/kai/mozb/mozilla/chrome/src/nsChromeRegistry.cpp, line 648
###!!! ASSERTION: redundant multiplexed document?: 'docMapEntry->mString ==
nsnull', file /home/kai/mozb/mozilla/xpcom/io/nsFastLoadFile.cpp, line 1398
Break: at file /home/kai/mozb/mozilla/xpcom/io/nsFastLoadFile.cpp, line 1398

Comment 20

[Kai Engert (:KaiE:)]

If David is the only person who can reproduce this bug, I would agree it should
not block the next release. Moreover because it's obviously contained in 1.5,
too. If we don't need what causes it, no need to stop 1.6 IMHO.

David, maybe you suffer from a corrupted certificate database.
Could you please make a test?
Quit Mozilla
Find your profile directory.
Move your *.db files to another directory.
Start Mozilla.
Can you still reproduce the crash?
(Once you are done testing, quit Mozilla again and restore your *.db files).

If you can stop Mozilla from crashing by moving these files away, it's not a
Mozilla bug, but a good old NSS database inconsistency :-(

Comment 21

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: cleanup patch, unrelated to crash (checked in 2005-02-23 12:10 -0800)

Either patch "changes the logic" in the case where we used to crash, but you
have to do that.  Neither approach changes anything in a case where we didn't
crash before.

I agree that it shouldn't be possible to end up with null.  (And I looked
through the nsSupportsArray code as well, and it looks ok.)

However, this code is more complex than it needs to be in at least 2 ways
(two-step ElementAt/QI instead of do_QueryElementAt, multiple insertion calls)
and the indentation is highly misleading (count++ is indented too far, right
after an if).  This patch cleans up those issues, if you're interested.

Comment 22

[David E. Ross]

Help me.  

I created a new, test profile and left as Mozilla created it.  The problem
report in this bug seems to not happen.  So perhaps I do indeed have corrupted
certificate files.  

In the test profile, I have only three .db files: cert8.db, key3.db, and
secmod.db.  In my usual profile, I have five .db files: those three plus
cert7.db and signed0.db.  I can replace the files in my usual profile with those
setup when I created the test profile, but what should I do about the two extra
files?  

By the way, the problem seems to be associated with trying to file a complaint
about a telemarketer at <http://www.ftc.gov/>.  They have something wrong there
that causes them not to recognize the CA for their certificate.  When I get the
warning about an unrecognized CA and try to view my CA certificates, that is
when Mozilla crashes (even if I try to open Certificate Manager later in the
session, when I am no longer at the FTC Web site).  The problem at the FTC Web
site goes away if I first go to another secure Web site that uses the same CA
certificate.  I figured that problem is an FTC Web site problem and not a
Mozilla problem, so I did not write a bug report on it.  

Comment 23

[Brendan Eich [:brendan]]

Kai, how about getting dbaron's attachment 136693 [details] [diff] [review] in for 1.7alpha?

/be

Comment 24

[David E. Ross]

Having performed a more thorough and organized test using a new, vanilla
profile, I conclude that the suggestion in comment #20 -- that I might have a
corrupted certificate database -- is indeed correct.  However, that should still
not cause Mozilla to crash.  Instead, Mozilla should detect something is amiss
and (at worst) gracefully terminate.  Thus, I believe a bug does indeed exist. 
However, since the bug appears only in a pathological environment, the severity
of this bug should be reduced to "Normal" from "Critical".  

I have received no response to my plea for help in the first half of my comment
#22.  I do not want to abandon a tailored profile that otherwise works well. 
Please advise me on correcting my .db files.  

Comment 25

[Nelson Bolyard (seldom reads bugmail)]

David, 

cert7.db was superseded by cert8.db in moz 1.3 (IIRC) and is no longer needed,
unless you go back to an older version of mozilla.  IIRC, signed0.db had 
something to do with signed jar files used by Communicator 4.x and is not used
by mozilla.

Comment 26

[Kai Engert (:KaiE:)]

Thanks for confirming the crash was caused by a corrupted cert db.

I know it's not a satisfactory answer, but as far as I know, we don't have a
mechanism to detect a corruption in the NSS cert database. I'm marking this bug
as invalid at the PSM level. The real fix would be to make the NSS database
failsafe against corruptions. It's known this would be good to have, but it's
difficult to do.

Comment 27

[Kai Engert (:KaiE:)]

Comment on attachment 136693 [details] [diff] [review]
cleanup patch, unrelated to crash (checked in 2005-02-23 12:10 -0800)

David, if you want to drive this cleanup patch in, please go ahead.
r=kaie

Comment 28

[OstGote!]

Had just several crashes with Mozilla 1.8b build 2005012606 on WinNT4 as I tried
to view the imported certificate directly after the import. Crash with the same
stack, see TB ids TB3318589K, TB3318637E, TB3342019G. For previously imported
certs I did not crash. After restart (of Mozilla) after 1st time crash I can
view the certs. Tried comment 20, after import I could directly view the cert
without crash. So my database has some inconsistencies, but the crash appears
not always only for the first time to view until restart.

Similiar thing is Bug 275892.

Comment 29

[Nelson Bolyard (seldom reads bugmail)]

I don't see any real evidence of a "corrupted" cert DB here.
I haven't seen any evidence that any correction to NSS's DB is needed.
I see only a crash with a null pointer in PSM code.

It may be that David's cert DB contained an invalid cert, or 
a cert that didn't contain some name attribute that PSM expected,
which resulted in some function returning NULL for a name attribute,
and then PSM stumbled over that NULL.  But the presence of odd certs 
doesn't constitute "corruption" of a cert DB.  

There have been several patches attached here, at least one of which
adds a null pointer check that avoids the crash.  Given that no-one
is apparently left who will dig further into this issue, I think it's
time to check in the null pointer check, and then call it fixed,
despite comment 9 and comment 12, which seem reluctant to add null
pointer checks.

Comment 30

[timeless]

dbaron: do you plan to check this in?

Comment 31

[OstGote!]

Crashed again with Mozilla 2005030706 on WinNT4 while I tried to add a second
certificate (first one ok, while importing the second one crash).

TB4217503M

Stack Signature	 nsCertTree::CmpByCrit da92bade
Product ID	MozillaTrunk
Build ID	2005030706
Trigger Time	2005-03-08 23:43:36.0
Platform	Win32
Operating System	Windows NT 4.0 build 1381
Module	pipnss.dll + (0001cbe6)
URL visited	N/A
User Comments	added 2 security certificates
Since Last Crash	2100 sec
Total Uptime	25712 sec
Trigger Reason	Access violation
Source File, Line No.
c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/security/manager/ssl/src/nsCertTree.cpp,
line 992
Stack Trace 	
nsCertTree::CmpByCrit 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/security/manager/ssl/src/nsCertTree.cpp,
line 992]
nsCertTree::CmpBy 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/security/manager/ssl/src/nsCertTree.cpp,
line 1026]
nsCertTree::CmpCACert 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/security/manager/ssl/src/nsCertTree.cpp,
line 1047]
nsCertTree::GetCertsByType 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/security/manager/ssl/src/nsCertTree.cpp,
line 334]
nsCertTree::LoadCerts 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/security/manager/ssl/src/nsCertTree.cpp,
line 391]
XPCWrappedNative::CallMethod 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/js/src/xpconnect/src/xpcwrappednative.cpp,
line 2067]
XPC_WN_CallMethod 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/js/src/xpconnect/src/xpcwrappednativejsops.cpp,
line 1287]
js_Invoke 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/js/src/jsinterp.c,
line 1293]
js_Interpret 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/js/src/jsinterp.c,
line 3568]
js_Invoke 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/js/src/jsinterp.c,
line 1313]
js_InternalInvoke 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/js/src/jsinterp.c,
line 1390]
JS_CallFunctionValue 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/js/src/jsapi.c, line
3804]
nsJSContext::CallEventHandler 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/dom/src/base/nsJSEnvironment.cpp,
line 1384]
nsJSEventListener::HandleEvent 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/dom/src/events/nsJSEventListener.cpp,
line 184]
nsEventListenerManager::HandleEventSubType 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/content/events/src/nsEventListenerManager.cpp,
line 1529]
nsEventListenerManager::HandleEvent 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/content/events/src/nsEventListenerManager.cpp,
line 1626]
nsXULElement::HandleDOMEvent 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/content/xul/content/src/nsXULElement.cpp,
line 2046]
PresShell::HandleDOMEventWithTarget 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/layout/base/nsPresShell.cpp,
line 6152]
nsButtonBoxFrame::MouseClicked 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/layout/xul/base/src/nsButtonBoxFrame.cpp,
line 177]
nsButtonBoxFrame::HandleEvent 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/layout/xul/base/src/nsButtonBoxFrame.cpp,
line 146]
PresShell::HandleEventInternal 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/layout/base/nsPresShell.cpp,
line 6117]
PresShell::HandleEventWithTarget 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/layout/base/nsPresShell.cpp,
line 5962]
nsEventStateManager::CheckForAndDispatchClick 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/content/events/src/nsEventStateManager.cpp,
line 2961]
nsEventStateManager::PostHandleEvent 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/content/events/src/nsEventStateManager.cpp,
line 1946]
PresShell::HandleEventInternal 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/layout/base/nsPresShell.cpp,
line 6125]
PresShell::HandleEvent 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/layout/base/nsPresShell.cpp,
line 5900]
nsViewManager::HandleEvent 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/view/src/nsViewManager.cpp,
line 2497]
nsViewManager::DispatchEvent 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/view/src/nsViewManager.cpp,
line 2217]
HandleEvent 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/view/src/nsView.cpp,
line 174]
nsWindow::DispatchEvent 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/widget/src/windows/nsWindow.cpp,
line 1127]
nsWindow::DispatchMouseEvent 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/widget/src/windows/nsWindow.cpp,
line 5491]
ChildWindow::DispatchMouseEvent 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/widget/src/windows/nsWindow.cpp,
line 5750]
nsWindow::WindowProc 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/widget/src/windows/nsWindow.cpp,
line 1419]
USER32.dll + 0x124c (0x77e7124c)

Comment 32

[Chuck Wade]

Folks,

I have just run into a what appears to be a similar problem with Mozilla 1.7.8
on the Mac OS (10.3.8). I have a moderately large cert database with certs going
back many years for S/MIME usage. Recently, Mozilla crashes every time I attempt
to "Manage Certificates." If I use other user profiles, I do not have the
problem and can "Manage Certificates." However, I need to be able to manage
certs in my production profile. Note, all other S/MIME usage appears to be
working with my production profile, and I can send signed and encrypted email
messages using the certs in my production profile. Note that I've sent a bunch
of TalkBack reports with <[e-mail]> as the email address.

I have restored my key3.db and cert8.db files from backups, trying older and
older versions. After going back about 2 months, I am still unable to Manage
Certificates with the restored files. I'm beginning to think the problem has
nothing to do with these two database files. Could it possibly be XUL.mfasl that
is corrupted? However, I tried restoring backups of this file as well.

One item of interest, I noticed this problem after receiving an email message
from someone who was running their own CA and had a self-signed cert. The
signature was invalid, and the sender sent me their root CA cert. As an
experiment, I tried opening the attachment (.p12) in Mozilla directly. It didn't
appear to do anything. I then went to import the cert directly, and that's when
I noticed that any attempt to activate "Manage Certificates" would crash Mozilla.

Aside, I've also tried the "chrome://pippki/content/certManager.xul" trick, and
this also crashes Mozilla. 

This is pretty frustrating. I can be reached at 508 625-1137 if anyone wants to
use the old fashioned approach.

Comment 33

[Kai Engert (:KaiE:)]

I have really no idea how this could happen, and I never saw this crash.
But a lot of people seem to crash at the same place.

I suggest we add some null checks.

Comment 34

[Kai Engert (:KaiE:)]

Attachment: safety patch, can't hurt

I propose we check this patch in to both trunk and 170 branch, it can't do any
harm.

Comment 35

[sairuh (rarely reading bugmail)]

(In reply to comment #32) 
> I have just run into a what appears to be a similar problem with Mozilla 1.7.8
> on the Mac OS (10.3.8). I have a moderately large cert database with certs going
> back many years for S/MIME usage. Recently, Mozilla crashes every time I attempt
> to "Manage Certificates." If I use other user profiles, I do not have the
> problem and can "Manage Certificates." However, I need to be able to manage
> certs in my production profile. Note, all other S/MIME usage appears to be
> working with my production profile, and I can send signed and encrypted email
> messages using the certs in my production profile. Note that I've sent a bunch
> of TalkBack reports with <[e-mail]> as the email address.

Chuck, I haven't been able to find incidents with your email address (I copied
the address you provided above), however I found this incident:

http://talkback-public.mozilla.org/talkback/fastfind.jsp?search=2&type=iid&id=5903010

is this the talkback report you had sent?

Stack Signature	nsCertTree::CmpByCrit(nsIX509Cert*, CompareCacheHashEntry*,
nsIX509Cert*, CompareCacheHashEntry*, nsCertTree::sortCriterion,() 37f8d955
Product ID	Firefox10
Build ID	2005051112
Trigger Time	2005-05-17 06:55:44.0
Platform	MacOSX
Operating System	Darwin 7.9.0
Module	firefox-bin + (00684604)

User Comments: Attempting to "manage certificates" via the advanced preferences
setup window. Cannot even display current certificate database; firefox crashes.
This happened and was reported w/firefox 1.0.3 and 1.0.2, all on OS X 10.3.9
Since Last Crash	30778 sec
Total Uptime	30778 sec
Trigger Reason	SIGSEGV: Segmentation Violation: (signal 11)
Source File, Line No.
/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/security/manager/ssl/src/nsCertTree.cpp,
line 1000
Stack Trace 	
nsCertTree::CmpByCrit(nsIX509Cert*, CompareCacheHashEntry*, nsIX509Cert*,
CompareCacheHashEntry*, nsCertTree::sortCriterion,() 
[/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/security/manager/ssl/src/nsCertTree.cpp,
line 1000]
nsCertTree::CmpBy(void*, nsIX509Cert*, nsIX509Cert*, nsCertTree::sortCriterion,
nsCertTree::sortCriterion, nsCertTree::sortCriterion)() 
[/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/security/manager/ssl/src/nsCertTree.cpp,
line 1035]
nsCertTree::GetCertsByTypeFromCertList() 
[/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/security/manager/ssl/src/nsCertTree.cpp,
line 704]
nsCertTree::LoadCertsFromCache() 
[/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/security/manager/ssl/src/nsCertTree.cpp,
line 371]
_XPTC_InvokeByIndex()   XPCWrappedNative::CallMethod(XPCCallContext&,
XPCWrappedNative::CallMode)() 
[/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/js/src/xpconnect/src/xpcwrappednative.cpp,
line 2033]
XPC_WN_CallMethod() 
[/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/js/src/xpconnect/src/xpcwrappednativejsops.cpp,
line 1781]
js_Invoke() 
[/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/js/src/jsinterp.c,
line 955]
js_Interpret() 
[/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/js/src/jsinterp.c,
line 3002]
js_Invoke() 
[/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/js/src/jsinterp.c,
line 972]
js_InternalInvoke() 
[/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/js/src/jsinterp.c,
line 1050]
JS_CallFunctionValue() 
[/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/js/src/jsapi.c,
line 3705]
nsJSContext::CallEventHandler() 
[/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/dom/src/base/nsJSEnvironment.cpp,
line 1299]
nsJSEventListener::HandleEvent() 
[/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/dom/src/events/nsJSEventListener.cpp,
line 177]
nsEventListenerManager::HandleEventSubType() 
[/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/content/events/src/nsEventListenerManager.cpp,
line 710]
nsEventListenerManager::HandleEvent() 
[/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/content/events/src/nsEventListenerManager.cpp,
line 1516]
GlobalWindowImpl::HandleDOMEvent() 
[/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/dom/src/base/nsGlobalWindow.cpp,
line 927]
DocumentViewerImpl::LoadComplete() 
[/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/content/base/src/nsDocumentViewer.cpp,
line 704]
nsDocShell::EndPageLoad() 
[/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/docshell/base/nsDocShell.cpp,
line 4609]
nsWebShell::EndPageLoad() 
[/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/docshell/base/nsWebShell.cpp,
line 430]
nsDocShell::OnStateChange() 
[/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/docshell/base/nsDocShell.cpp,
line 198]
nsDocLoaderImpl::FireOnStateChange() 
[/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/uriloader/base/nsDocLoader.cpp,
line 710]
nsDocLoaderImpl::doStopDocumentLoad() 
[/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/uriloader/base/nsDocLoader.cpp,
line 868]
nsDocLoaderImpl::DocLoaderIsEmpty() 
[/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/uriloader/base/nsDocLoader.cpp,
line 771]
nsDocLoaderImpl::OnStopRequest() 
[/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/uriloader/base/nsDocLoader.cpp,
line 698]
nsLoadGroup::RemoveRequest() 
[/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/netwerk/base/src/nsLoadGroup.cpp,
line 710]
imgRequestProxy::RemoveFromLoadGroup() 
[/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/modules/libpr0n/src/imgRequestProxy.cpp,
line 158]
imgRequest::OnStopRequest() 
[/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/modules/libpr0n/src/imgRequest.cpp,
line 692]
nsJARChannel::OnStopRequest() 
[/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/netwerk/protocol/jar/src/nsJARChannel.cpp,
line 607]
nsInputStreamPump::OnStateStop() 
[/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/netwerk/base/src/nsInputStreamPump.cpp,
line 607]
nsInputStreamPump::OnInputStreamReady() 
[/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/netwerk/base/src/nsInputStreamPump.cpp,
line 339]
nsInputStreamReadyEvent::EventHandler()
PL_HandleEvent() 
[/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/xpcom/threads/plevent.c,
line 674]
PL_ProcessPendingEvents() 
[/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/xpcom/threads/plevent.c,
line 608]
_md_EventReceiverProc() 
[/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/xpcom/threads/plevent.c,
line 1578]
HIToolbox.145.0.0 + 0x1fa0 (0x92881fa0)
HIToolbox.145.0.0 + 0x2214 (0x92882214)
HIToolbox.145.0.0 + 0x6694 (0x92886694)
HIToolbox.145.0.0 + 0x12d2c (0x92892d2c)
HIToolbox.145.0.0 + 0x205c (0x9288205c)
HIToolbox.145.0.0 + 0x2214 (0x92882214)
HIToolbox.145.0.0 + 0x146bc (0x928946bc)
HIToolbox.145.0.0 + 0x185d8 (0x928985d8)
HIToolbox.145.0.0 + 0x28718 (0x928a8718)
HIToolbox.145.0.0 + 0x8d88 (0x92888d88)
HIToolbox.145.0.0 + 0x9064 (0x92889064)
HIToolbox.145.0.0 + 0x1c9f0 (0x9289c9f0)
HIToolbox.145.0.0 + 0x2d708 (0x928ad708)
nsMacMessagePump::GetEvent() 
[/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/widget/src/mac/nsMacMessagePump.cpp,
line 407]
nsAppShell::GetNativeEvent() 
[/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/widget/src/mac/nsAppShell.cpp,
line 221]
nsXULWindow::ShowModal() 
[/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/xpfe/appshell/src/nsXULWindow.cpp,
line 363]
nsWindowWatcher::OpenWindowJS() 
[/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/embedding/components/windowwatcher/src/nsWindowWatcher.cpp,
line 570]
nsWindowWatcher::OpenWindow() 
[/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/embedding/components/windowwatcher/src/nsWindowWatcher.cpp,
line 464]
GlobalWindowImpl::OpenInternal() 
[/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/dom/src/base/nsGlobalWindow.cpp,
line 1211]
GlobalWindowImpl::Open() 
[/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/dom/src/base/nsGlobalWindow.cpp,
line 202]
_XPTC_InvokeByIndex()   XPCWrappedNative::CallMethod(XPCCallContext&,
XPCWrappedNative::CallMode)() 
[/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/js/src/xpconnect/src/xpcwrappednative.cpp,
line 2033]
XPC_WN_CallMethod() 
[/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/js/src/xpconnect/src/xpcwrappednativejsops.cpp,
line 1781]
js_Invoke() 
[/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/js/src/jsinterp.c,
line 955]
js_Interpret() 
[/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/js/src/jsinterp.c,
line 3002]
js_Invoke() 
[/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/js/src/jsinterp.c,
line 972]
js_InternalInvoke() 
[/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/js/src/jsinterp.c,
line 1050]
JS_CallFunctionValue() 
[/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/js/src/jsapi.c,
line 3705]
nsJSContext::CallEventHandler() 
[/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/dom/src/base/nsJSEnvironment.cpp,
line 1299]
nsJSEventListener::HandleEvent() 
[/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/dom/src/events/nsJSEventListener.cpp,
line 177]
nsEventListenerManager::HandleEventSubType() 
[/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/content/events/src/nsEventListenerManager.cpp,
line 710]
nsEventListenerManager::HandleEvent() 
[/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/content/events/src/nsEventListenerManager.cpp,
line 1516]
nsXULElement::HandleDOMEvent() 
[/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/content/xul/content/src/nsXULElement.cpp,
line 2841]
PresShell::HandleDOMEventWithTarget() 
[/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/layout/html/base/src/nsPresShell.cpp,
line 6139]
nsButtonBoxFrame::MouseClicked() 
[/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/layout/xul/base/src/nsButtonBoxFrame.cpp,
line 65]
nsButtonBoxFrame::HandleEvent() 
[/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/layout/xul/base/src/nsButtonBoxFrame.cpp,
line 150]
PresShell::HandleEventInternal() 
[/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/layout/html/base/src/nsPresShell.cpp,
line 6102]
PresShell::HandleEventWithTarget() 
[/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/layout/html/base/src/nsPresShell.cpp,
line 5984]
nsEventStateManager::CheckForAndDispatchClick() 
[/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/content/events/src/nsEventStateManager.cpp,
line 2988]
nsEventStateManager::PostHandleEvent() 
[/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/content/events/src/nsEventStateManager.cpp,
line 141]
PresShell::HandleEventInternal() 
[/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/layout/html/base/src/nsPresShell.cpp,
line 710]
PresShell::HandleEvent() 
[/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/layout/html/base/src/nsPresShell.cpp,
line 5920]
nsViewManager::HandleEvent() 
[/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/view/src/nsViewManager.cpp,
line 61]
nsViewManager::DispatchEvent() 
[/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/view/src/nsViewManager.cpp,
line 2066]
HandleEvent() 
[/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/view/src/nsView.cpp,
line 78]
nsWindow::DispatchEvent() 
[/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/widget/src/mac/nsWindow.cpp,
line 2023]
nsWindow::DispatchWindowEvent() 
[/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/widget/src/mac/nsWindow.cpp,
line 2039]
nsWindow::DispatchMouseEvent() 
[/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/widget/src/mac/nsWindow.cpp,
line 2065]
nsMacEventHandler::HandleMouseUpEvent() 
[/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/widget/src/mac/nsMacEventHandler.cpp,
line 1677]
nsMacEventHandler::HandleOSEvent() 
[/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/widget/src/mac/nsMacEventHandler.cpp,
line 551]
nsMacWindow::DispatchEvent() 
[/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/widget/src/mac/nsMacWindow.cpp,
line 282]
nsMacMessagePump::DispatchOSEventToRaptor() 
[/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/widget/src/mac/nsMacMessagePump.cpp,
line 589]
nsMacMessagePump::DoMouseUp() 
[/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/widget/src/mac/nsMacMessagePump.cpp,
line 815]
nsMacMessagePump::DispatchEvent() 
[/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/widget/src/mac/nsMacMessagePump.cpp,
line 447]
nsAppShell::DispatchNativeEvent() 
[/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/widget/src/mac/nsAppShell.cpp,
line 232]
nsXULWindow::ShowModal() 
[/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/xpfe/appshell/src/nsXULWindow.cpp,
line 710]
nsWindowWatcher::OpenWindowJS() 
[/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/embedding/components/windowwatcher/src/nsWindowWatcher.cpp,
line 570]
GlobalWindowImpl::OpenInternal() 
[/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/dom/src/base/nsGlobalWindow.cpp,
line 1211]
GlobalWindowImpl::OpenDialog() 
[/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/dom/src/base/nsGlobalWindow.cpp,
line 3565]
_XPTC_InvokeByIndex()   XPCWrappedNative::CallMethod(XPCCallContext&,
XPCWrappedNative::CallMode)() 
[/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/js/src/xpconnect/src/xpcwrappednative.cpp,
line 2033]
XPC_WN_CallMethod() 
[/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/js/src/xpconnect/src/xpcwrappednativejsops.cpp,
line 1781]
js_Invoke() 
[/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/js/src/jsinterp.c,
line 955]
js_Interpret() 
[/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/js/src/jsinterp.c,
line 3002]
js_Invoke() 
[/builds/tinderbox/Fx-Aviary1.0.1/Darwin_7.7.0_Depend/mozilla/js/src/jsinterp.c,
line 972]

Comment 36

[Daniel Veditz [:dveditz]]

(In reply to comment #32)
> Note that I've sent a bunch
> of TalkBack reports with <[e-mail]> as the email address.

6049588
6043481
6040342

Do we not get symbols for Mac builds? I'm not sure those help much.

Comment 37

[Darin Fisher]

Comment on attachment 184374 [details] [diff] [review]
safety patch, can't hurt

Do you want to perhaps change these to using NS_ENSURE_TRUE so that there is a
warning output in debug builds?

Comment 38

[Kai Engert (:KaiE:)]

Attachment: safety patch, including Darin's suggestion

Comment 39

[Nelson Bolyard (seldom reads bugmail)]

http://talkback-public.mozilla.org/talkback/fastfind.jsp?search=1&searchby=comments&match=contains&searchfor=certificates&vendor=All&product=All&platform=All
shows 17 similar crashes

Comment 40

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Comment on attachment 184380 [details] [diff] [review]
safety patch, including Darin's suggestion

a=shaver

Comment 41

[Caleb]

The last patch needs to be checked in!

Comment 42

[Caleb]

Patch was checked in by timeless. Marking as FIXED unless anyone objects.

Comment 43

[Chuck Wade]

(In reply to comment #42)
> Patch was checked in by timeless. Marking as FIXED unless anyone objects.

Unfortunately, I still have this crashing problem with 1.7.10, and just
submitted additional talkback reports. I see that others are having what appear
to be identical problems. I can confirm that the crash occurs with a specific
cert data base, but the impact on the user is enormous, especially someone who
has many years of s/mime email and a history of certs for multiple email
addresses renewed over many years.

I would be willing to try manual recovery techniques is someone could give me a
clue as to how to go about it. I'm not evey sure which file is the culprit, as
there seem to be required combinations of files.

Any help would be appreciated.

Comment 44

[Jaime Mitchell (use [e-mail] for email)]

(In reply to comment #43)
> Unfortunately, I still have this crashing problem with 1.7.10, and just
> submitted additional talkback reports. 

The patch was checked into the trunk therefore it is not in the 1.7 branch, you
need to download a nightly build of seamonkey or wait until seamonkey 1.0 is
released.

Comment 45

[Chuck Wade]

(In reply to comment #44)
> (In reply to comment #43)
> > Unfortunately, I still have this crashing problem with 1.7.10, and just
> > submitted additional talkback reports. 
> 
> The patch was checked into the trunk therefore it is not in the 1.7 branch, you
> need to download a nightly build of seamonkey or wait until seamonkey 1.0 is
> released.

OK, I just tried the alpha release of Seamonkey for Mac OS X. Unfortunately,
same problem. I crash every time I try to "manage certificates." I submitted a
talkback report as TB8003226G. 

What else can I try? I'm beginning to get a bit more desperate. Up until
recently, my cert database at least worked, but now I'm having trouble sending
encrypted email messages, even though I can still sign and decrypt messages I
receive. I can be reached at <[e-mail]>

Comment 46

[Chuck Wade]

(In reply to comment #44)
> (In reply to comment #43)
> > Unfortunately, I still have this crashing problem with 1.7.10, and just
> > submitted additional talkback reports. 
> 
> The patch was checked into the trunk therefore it is not in the 1.7 branch, you
> need to download a nightly build of seamonkey or wait until seamonkey 1.0 is
> released.

I have now confirmed that this problem exists with Firefox 1.0.6 (Mac OS
10.3.9). I submitted a Talkback report as TB8008570X. Is there a possibility
that a nightly build of Firefox might include a fix? As noted in an earlier
reply, the Aug 1 build for Seamonkey doesn't help either.

Bottom line: it does NOT look like this bug has been resolved. There are also
other reports against Firefox and Thunderbird from the past couple of months
that indicate others have a similar problem.

Comment 47

[timeless]

Incident ID: 8003226
Stack Signature	nsCertTree::CmpByCrit(nsIX509Cert*, CompareCacheHashEntry*,
nsIX509Cert*, CompareCacheHashEntry*, nsCertTree::sortCriterion,() ffe7253d
Product ID	MozillaTrunk
Build ID	2005080109
Trigger Time	2005-08-01 14:03:33.0
Platform	MacOSX
Operating System	Darwin 7.9.0
Module	libpipnss.dylib + (00033328)
URL visited	
User Comments	Attempted to open certificate manager. This consistently results
in a crash of all versions of Mozilla I've tried, incl. Seamonkey. It is unique
to my cert db, as new profiles do not have this problem. This is a serious
problem. See Bugzilla Bug 225034
Since Last Crash	217 sec
Total Uptime	217 sec
Trigger Reason	SIGSEGV: Segmentation Violation: (signal 11)
Source File, Line No.
/builds/tinderbox/Mozilla-Trunk/Darwin_6.8_Depend/mozilla/security/manager/ssl/src/nsCertTree.cpp,
line 998
Stack Trace 	
nsCertTree::CmpByCrit(nsIX509Cert*, CompareCacheHashEntry*, nsIX509Cert*,
CompareCacheHashEntry*, nsCertTree::sortCriterion,() 
[/builds/tinderbox/Mozilla-Trunk/Darwin_6.8_Depend/mozilla/security/manager/ssl/src/nsCertTree.cpp,
line 998]

Comment 48

[Chuck Wade]

(In reply to comment #46)
> (In reply to comment #44)
> > (In reply to comment #43)
> > > [snip...]

I just want to confirm that I have tested the "manage certificates" capability
in Thunderbird 1.0.6 with my cert database. I specifically let Thunderbird
import my complete email settings from my Mozilla profile, and yet I still
encounter a crash when I attempt to manage the imported database. I filed a
Talkback report as TB8123083G.

Again, I am willing to do further debugging and to also play around with some
code and custom utilities to gain further insight into this problem. However, I
have not found sufficient documentation on how the cert databases are
constructed to feel like I can just dive in. If someone wants to point me in the
right direction, I'll see if I can make a contribution.

...Chuck 

Comment 49

[Robert Relyea]

I have a patch for this problem.

The issue is the use of the PL_DHash* functions. It's possible that a given call
to PL_DHashOperate which adds a new entry may cause the hash table to expand,
and all the existing entries to be reallocated. PL_DHash does this by allocating
new memory, then copying the entries.

getCacheEntry() returns one of these hash entries. CmpBy() makes two consecutive
calls to getCacheEntry, then uses the returned entries for it's comparisons. If
the second entry call causes a new entry to be added to the table, and causes
the hash table to expand, the pointer to the first entry we retrieved will point
to freed memory. This requires a database with a fair number of entries (the
problem will not be reproduced by an empty database. On debug builds the crash
is more frequent (though still unpredictable) because the debug version of free
writes garbage into the freed memory. On optimize builds, the problem will only
be seen if something else alocates the memory before the CmpBy() functions are
done with it (making the problem much more rare on optimize builds and leading
to the problem only detected through talkback reports.

The fix is to make the usable entry a pointer in the hashtable entry, and return
that pointer. When the hashtable rebuilds it's entries, the pointer will be
copied to the new entry and not be disturbed. It will also reduce the space of
the hashtable itself, as the current entry size is 60 bytes.

Patch is forthcoming.

Comment 50

[Robert Relyea]

Attachment: Store the CacheEntry as a pointer in the hash table rather than just a value

Some notes: Now that the CacheEntry is stored as a pointer, it is possible for
the 'new' to fail. That means we have to check for this case before we use the
pointer in our callback routines.

Comment 51

[Robert Relyea]

Attachment: Same patch as 192540 except with more context to faciliate reviews.

Comment 52

[Brendan Eich [:brendan]]

Comment on attachment 192542 [details] [diff] [review]
Same patch as 192540 except with more context to faciliate reviews.

Bob, not sure from whom you were soliciting review, but I can sr -- see below.

>+CompareCacheHashEntryPtr::CompareCacheHashEntryPtr()
>+{
>+   entry = new CompareCacheHashEntry;
>+}
>+
>+CompareCacheHashEntryPtr::~CompareCacheHashEntryPtr()
>+{
>+   if (entry) {
>+        delete entry;
>+   }
>+}
>+

Yikes, Fibonacci indentation (3, 5)!  Looks like c-basic-offset is 2, so please
respect that.

> CompareCacheHashEntry::CompareCacheHashEntry()
> :key(nsnull)
> {
>   for (int i = 0; i < max_criterions; ++i) {
>     mCritInit[i] = PR_FALSE;
>   }
> }
> 
> PR_STATIC_CALLBACK(const void *)
> CompareCacheGetKey(PLDHashTable *table, PLDHashEntryHdr *hdr)
> {
>-  CompareCacheHashEntry *entry = NS_STATIC_CAST(CompareCacheHashEntry*, hdr);
>-  return entry->key;
>+  CompareCacheHashEntryPtr *entryPtr = NS_STATIC_CAST(CompareCacheHashEntryPtr*, hdr);
>+  if (!entryPtr || !entryPtr->entry) return NULL;

The pldhash code will never call this hook with a null hdr, so there is no need
to test !entryPtr.

Nit: then clause (the return NULL) on its own line.

Question: how can entryPtr->entry ever be null?  Oh, if new fails.  But can
that happen on modern OSes, and if so, does all code cope?

> PR_STATIC_CALLBACK(PRBool)
> CompareCacheMatchEntry(PLDHashTable *table, const PLDHashEntryHdr *hdr,
>                          const void *key)
> {
>-  const CompareCacheHashEntry *entry = NS_STATIC_CAST(const CompareCacheHashEntry*, hdr);
>-  return entry->key == key;
>+  const CompareCacheHashEntryPtr *entryPtr = NS_STATIC_CAST(const CompareCacheHashEntryPtr*, hdr);
>+  if (!entryPtr || !entryPtr->entry) return PR_FALSE;

Same comments here.

> PR_STATIC_CALLBACK(void)
> CompareCacheClearEntry(PLDHashTable *table, PLDHashEntryHdr *hdr)
> {
>-  CompareCacheHashEntry *entry = NS_STATIC_CAST(CompareCacheHashEntry*, hdr);
>-  entry->~CompareCacheHashEntry();
>+  CompareCacheHashEntryPtr *entryPtr = NS_STATIC_CAST(CompareCacheHashEntryPtr*, hdr);
>+  if (entryPtr) {

Again, pldhash code will never call this hook with null hdr.

> CompareCacheHashEntry *
> nsCertTree::getCacheEntry(void *cache, void *aCert)
> {
>   PLDHashTable &aCompareCache = *NS_REINTERPRET_CAST(PLDHashTable*, cache);
>-  return NS_STATIC_CAST(CompareCacheHashEntry*,
>+  CompareCacheHashEntryPtr *entryPtr = NS_STATIC_CAST(CompareCacheHashEntryPtr*,
>                         PL_DHashTableOperate(&aCompareCache, aCert,
>                                              PL_DHASH_ADD));

Style suggestion: put the NS_STATIC_CAST on its own line, indented by
c-basic-offset (2) spaces, then underhang any arguments to that macro that
don't fit, and do the same in nested sense for PL_DHashTableOperate's overlong
arg list.

If you want to request me to sr, put up a new patch and set the request flag to
have [e-mail] as requestee.

/be

Comment 53

[Robert Relyea]

Thanks for the review brendan. I'll add you to sr the updated patch (I prefer
someone who's familiar with the Hash code;).

I'll fix the formatting stuff (I try to keep the style of the existing file, but
NSS indentation style is pretty well ingrained;).

I presumed new could fail to get space. At least the rest of the code that
relies on ->entry checks (you see it all here).

There are null checks for NULL cache entries (patch in this bug).

new patch shortly..

Comment 54

[Wan-Teh Chang]

Comment on attachment 192542 [details] [diff] [review]
Same patch as 192540 except with more context to faciliate reviews.

Good job.

This patch looks good.	Some comments.

>+CompareCacheHashEntryPtr::CompareCacheHashEntryPtr()
>+{
>+   entry = new CompareCacheHashEntry;
>+}
>+
>+CompareCacheHashEntryPtr::~CompareCacheHashEntryPtr()
>+{
>+   if (entry) {
>+        delete entry;
>+   }
>+}

Mozilla code indents by two spaces.

> PR_STATIC_CALLBACK(const void *)
> CompareCacheGetKey(PLDHashTable *table, PLDHashEntryHdr *hdr)
> {
>-  CompareCacheHashEntry *entry = NS_STATIC_CAST(CompareCacheHashEntry*, hdr);
>-  return entry->key;
>+  CompareCacheHashEntryPtr *entryPtr = NS_STATIC_CAST(CompareCacheHashEntryPtr*, hdr);
>+  if (!entryPtr || !entryPtr->entry) return NULL;
>+  return entryPtr->entry->key;
> }

I believe that entryPtr (which is equal to hdr) cannot be NULL
because the hashtable code only calls this callback function
with a valid hdr.  This is why the original code assumes entry
is not NULL.  Same comment applies to CompareCacheMatchEntry,
CompareCacheInitEntry, and CompareCacheClearEntry.

> PR_STATIC_CALLBACK(PRBool)
> CompareCacheInitEntry(PLDHashTable *table, PLDHashEntryHdr *hdr,
>                      const void *key)
> {
...
>+  new (hdr) CompareCacheHashEntryPtr();

This line seems to be a memory leak because we don't store the
return value of the new operator.


> PR_STATIC_CALLBACK(void)
> CompareCacheClearEntry(PLDHashTable *table, PLDHashEntryHdr *hdr)
> {
...
>+    entryPtr->~CompareCacheHashEntryPtr();

"delete entryPtr;" ?

>+struct CompareCacheHashEntryPtr : PLDHashEntryHdr {
>+  CompareCacheHashEntryPtr();
>+  ~CompareCacheHashEntryPtr();
>+   CompareCacheHashEntry *entry;
>+};

The declaration of entry is off by one space.  (Sorry, since I
already saw it ...)

Comment 55

[Christian :Biesinger (don't email me, ping me on IRC)]

new can indeed fail to get space, at least on vc6. timeless actually runs into
such cases.

Comment 56

[Robert Relyea]

wtc re:

new (hdr) XXXXXX;

and 

entryPtr->~XXXXX

new (hdr) XXXXX takes the pointer header and calls the constructor without
allocating new memory. We 'know' that hdr already has the right amount of space
because the Hash code has allocated it for us (we told it the size of our entry
alread). [The comment was good, though because it lead to to a bug in the patch.
I need to pass in sizeof(CompareCacheHashEntryPtr) to the table init rather than
sizeof(CompareCacheHashEntry). The bug isn't fatal because  the former is
smaller than the latter... it was just wasteful.

entryPtr->~XXXXX differs from 'delete entryPtr' by calling the destructor
without actually freeing entryPtr (which was alocated the the hash code, and
will be freed by the hash code).

These functions are sort of pro-forma calls which many mozilla components use to
make a hash header into a C++ object.

Comment 57

[Brendan Eich [:brendan]]

(In reply to comment #54)
> >+  new (hdr) CompareCacheHashEntryPtr();
> 
> This line seems to be a memory leak because we don't store the
> return value of the new operator.

Placement new uses the storage passed in parens to the new operator, so no need
to store or even use the return value.

Bob, my point (badly expressed) about out-of-memory was that it's a rare, hard
case -- so let's not add null checks where they are not needed.  If the only
place you new an EntryPtr struct is in the initEntry hook, and if that hook
fails when new returns null, then you do not need null entryPtr->entry tests
anywhere else, because pldhash takes care to abort the new entry when initEntry
fails (returns false).

/be

Comment 58

[Robert Relyea]

Attachment: Incorporate wtc and brendan's comments

This should have all the formatting changes, as well as removal of redundant
NULL checks.

bob

Comment 59

[Brendan Eich [:brendan]]

Comment on attachment 192556 [details] [diff] [review]
Incorporate wtc and brendan's comments

Looks great, thanks -- sr=me.

/be

Comment 60

[Brendan Eich [:brendan]]

Comment on attachment 192556 [details] [diff] [review]
Incorporate wtc and brendan's comments

Argh, bugzilla does not detect flag collisions.

/be

Comment 61

[Robert Relyea]

Comment on attachment 192556 [details] [diff] [review]
Incorporate wtc and brendan's comments

Since this shows up in talkback, I think it's a pretty good candidate for 1.8.

The code in the patch is well contained, and I've been running it under
xulrunner with multiple databases loaded with no problem.

Comment 62

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 192556 [details] [diff] [review]
Incorporate wtc and brendan's comments

1.8 has branched, so this will need to land both on the trunk (open now) and
the MOZILLA_1_8_BRANCH (should be open in a few hours, see Mozilla1.8
tinderbox)

Comment 63

[amano]

Hmm. Althogh there is an approval, this hasn't been checked in on the branch
until now.

Comment 64

[Robert Relyea]

I went into the trunk today. Friday was to late for me to wait for Tinderbox,
and I wanted to do a verification build on the branch (I tested it against the
trunk).

bob

Comment 65

[Robert Relyea]

Patch checked into the trunk and the 1.8 branch.

Trunk:
/mozilla/security/manager/ssl/src/nsCertTree.cpp
new revision: 1.43; previous revision: 1.42
/mozilla/security/manager/ssl/src/nsCertTree.h
new revision: 1.11; previous revision: 1.10


MOZILLA_1_8_BRANCH:
/mozilla/security/manager/ssl/src/nsCertTree.cpp
new revision: 1.42.4.1; previous revision: 1.42
/mozilla/security/manager/ssl/src/nsCertTree.h
new revision: 1.10.28.1; previous revision: 1.10

Comment 66

[Kai Engert (:KaiE:)]

Kudos to Bob for finding the real issue!!!

If I understand correctly, a simpler fix, avoiding the additional allocation
layer, could have been:

    CompareCacheHashEntry *ace = getCacheEntry(cache, a);
    CompareCacheHashEntry *bce = getCacheEntry(cache, b);
+   ace = getCacheEntry(cache, a);

Comment 67

[Jay Patel [:jay]]

v.fixed on trunk and branch per Talkback data.

Comment 68

[Nelson Bolyard (seldom reads bugmail)]

Bob, please set a target milestone on this bug to show in what release it was fixed.

Comment 69

[timeless]

*** Bug 311424 has been marked as a duplicate of this bug. ***

Comment 70

[Bruce Robson]

This bug exists in Mozilla version 1.7.12 (see bug 311424) which was built on
2005-09-15 (about a month after the checkins on the trunk and 1.8 branchs).

Are further 1.7 branch releases planned ?

Should the fix also be made to the 1.7 branch ?

Comment 71

[Kai Engert (:KaiE:)]

Comment on attachment 192556 [details] [diff] [review]
Incorporate wtc and brendan's comments

Sounds fine to me, but Mozilla drivers hat to make the decision, whether it
makes sense to take this stability patch to the branch.

At least the patch applies cleanly, no separate patch is required.

Comment 72

[Wan-Teh Chang]

Comment on attachment 192556 [details] [diff] [review]
Incorporate wtc and brendan's comments

Let's also request aviary1.0.8 approval.

In general only security vulnerability fixes go into
Firefox/Thunderbird 1.0.x and Mozilla 1.7.x releases.
But this bug is a common crash among certificate manager
users.	The fix has been on the Mozilla trunk and MOZILLA_1_8_BRANCH
since 8/15.

Comment 73

[Bruce Robson]

It’s almost two months since I reported that the bug still existed in the 1.7.12 and asked if this fix should also be made to the 1.7 branch.

As there hasn't been a final decision made on whether this fix should also be made to the 1.7 branch, I’ve set the blocking 1.7.13 flag to “?” to remind people that this question has been asked.

p.s. My understanding (from http://www.mozilla.org/seamonkey-transition.html) is that the intention is to maintain a long-lived, stable 1.7.x version of Seamonkey. I also believe that 1.7 can not be called stable when it continues to crash due to this bug.

Comment 74

[Daniel Veditz [:dveditz]]

Comment on attachment 192556 [details] [diff] [review]
Incorporate wtc and brendan's comments

a=dveditz for aviary1.0.x and mozilla.1.7 branches

Comment 75

[Kai Engert (:KaiE:)]

I checked this in to MOZILLA_1_7_BRANCH 

Checking in nsCertTree.cpp;
/cvsroot/mozilla/security/manager/ssl/src/nsCertTree.cpp,v  <--  nsCertTree.cpp
new revision: 1.38.2.1; previous revision: 1.38
done
Checking in nsCertTree.h;
/cvsroot/mozilla/security/manager/ssl/src/nsCertTree.h,v  <--  nsCertTree.h
new revision: 1.9.16.1; previous revision: 1.9
done


and to AVIARY_1_0_1_20050124_BRANCH.

Checking in nsCertTree.cpp;
/cvsroot/mozilla/security/manager/ssl/src/nsCertTree.cpp,v  <--  nsCertTree.cpp
new revision: 1.38.16.1; previous revision: 1.38
done
Checking in nsCertTree.h;
/cvsroot/mozilla/security/manager/ssl/src/nsCertTree.h,v  <--  nsCertTree.h
new revision: 1.9.30.1; previous revision: 1.9
done

Comment 76

[Marcia Knous [:marcia]]

verifying fixed on the Mac using Mozilla 1.7.12
Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.7.12) Gecko/20060209 as well as Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.7.12) Gecko/20060209 Firefox/1.0.7.  I don't crash when following the steps to reproduce. Adding verified keywords. Note that builds have not yet been renamed to 1.0.8 and 1.7.13.

Comment 77

[Tracy Walker [:tracy]]

Verified On Thunderbird version 1.0.8 (20060317)