iframes pointing to signed XUL crashes browser

RESOLVED FIXED in mozilla1.7alpha

Status

()

Core
Security
P1
critical
RESOLVED FIXED
14 years ago
14 years ago

People

(Reporter: Nick Mitchell, Assigned: Christopher Aillon (sabbatical, not receiving bugmail))

Tracking

({crash, regression})

Trunk
mozilla1.7alpha
crash, regression
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

14 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.6) Gecko/20040113
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.6) Gecko/20040113

When I go to the given URL, the page loads correctly, but if I then reload or
close Mozilla it crashes with the message "mozilla.exe has caused errors and
will be terminated..."

In the testcase (an XUL page)there are two frames, one points to a normal XUL
page, the other points to a signed XUL page. The signing is done using signtool
and using a certificate we created ourselves. This problem seems to have come up
with version 1.6


Reproducible: Always

Steps to Reproduce:
1. load up testcase (page loads fine)
2. click reload
Actual Results:  
Mozilla crashes - error message is displayed

Expected Results:  
displayed the page again
crashes here also - after having reloaded the URL a few times...

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7a) Gecko/20040120

But there seems to be no talkback agent - wonder why
Keywords: crash
There's no talkback because talkback is not set up yet.

caillon, this is yours -- the code in nsNSSComponent::VerifySignature needs to
addref aPrincipal after assigning it.
Assignee: general → security-bugs
Status: UNCONFIRMED → NEW
Component: Browser-General → Security: General
Ever confirmed: true
Keywords: regression
OS: Windows 2000 → All
Hardware: PC → All
Regression from bug 83536
Assignee: security-bugs → caillon

Comment 4

14 years ago
This crashes Firebird as well.

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7a) Gecko/20040121 Firebird/0.8.0+
Created attachment 139710 [details] [diff] [review]
Fix the crash; and another one.

There was another (not my fault) potential crash which I plugged, as well as
did some minor cleanup.
Created attachment 139711 [details] [diff] [review]
Better

Sigh.  So they cast an enum containing 3 things to a PRBool (which is typedefed
to a PRIntn) so we really can't in good faith just check if (!bool).  This
should do the trick.
Attachment #139710 - Attachment is obsolete: true
Attachment #139711 - Flags: superreview?(jst)
Attachment #139711 - Flags: review?(jst)
+  // SEC_PKCS7VerifyDetachedSignature returns a SECStatus cast to PRBool,
+  // so we have to explicitly check |if (verified != PR_TRUE)| here, rather
+  // than check |if (!verified)|
+  if (verified != PR_TRUE) {

so um...
according to
http://lxr.mozilla.org/seamonkey/source/security/nss/lib/util/seccomon.h#99 a
SECStatus is never PR_TRUE (which is defined as 1). so this comparison seems to
be always true?
Comment on attachment 139711 [details] [diff] [review]
Better

+  PRBool verified = SEC_PKCS7VerifyDetachedSignature(p7_info,
+						      certUsageObjectSigner,
+						      &digest, HASH_AlgSHA1,
+						      PR_TRUE);
+  // SEC_PKCS7VerifyDetachedSignature returns a SECStatus cast to PRBool,
+  // so we have to explicitly check |if (verified != PR_TRUE)| here, rather
+  // than check |if (!verified)|

Oh, how nice! Is there a bug on file on that?

r+sr=jst
Attachment #139711 - Flags: superreview?(jst)
Attachment #139711 - Flags: superreview+
Attachment #139711 - Flags: review?(jst)
Attachment #139711 - Flags: review+
Created attachment 139953 [details] [diff] [review]
What was checked in

I decided to undo the changes to the SECStatus/PRBool thing.  I don't want that
cvs blame.  :-)
Checked in at 01/26/2004 21:01 PST.
Status: NEW → RESOLVED
Last Resolved: 14 years ago
Priority: -- → P1
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.7alpha
*** Bug 229845 has been marked as a duplicate of this bug. ***
*** Bug 234622 has been marked as a duplicate of this bug. ***
You need to log in before you can comment on or make changes to this bug.