343682 - crash in libcrmf with ecc

Status: RESOLVED FIXED

(NSS :: Libraries, defect)

Description

[Kai Engert (:KaiE:)]

use 1.8 branch sources and apply patch v4 from bug 326159 and build firefox

go to the test page http://kuix.de/misc/test3/crmf-ecc.php and click the first button (ec crmf dual-use)

this crashes with the stack as shown in the attached text file

Comment 1

[Kai Engert (:KaiE:)]

Attachment: crash stack

Comment 2

[Robert Relyea]

Untested patch:
Index: crmfpop.c
===================================================================
RCS file: /cvsroot/mozilla/security/nss/lib/crmf/crmfpop.c,v
retrieving revision 1.6
diff -u -r1.6 crmfpop.c
--- crmfpop.c   22 May 2006 22:38:56 -0000      1.6
+++ crmfpop.c   5 Jul 2006 22:54:15 -0000
@@ -185,8 +185,8 @@
                  SECKEYPrivateKey   *inKey,
                  SECAlgorithmID     *inAlgId)
 {
-    SECItem                      derCertReq;
-    SECItem                      certReqSig;
+    SECItem                      derCertReq = { siBuffer, NULL, 0 };
+    SECItem                      certReqSig = { siBuffer, NULL, 0 };
     SECStatus                    rv = SECSuccess;

     rv = crmf_encode_certreq(certReq, &derCertReq);

Comment 3

[Kai Engert (:KaiE:)]

(In reply to comment #2)
> Untested patch:
> Index: crmfpop.c

No luck with this patch, still crashes with same stack.

Comment 4

[Kai Engert (:KaiE:)]

(In reply to comment #3)
> 
> No luck with this patch, still crashes with same stack.

Sorry, I made a mistake when testing the patch, it fixes the crash just fine, thanks Bob!

Comment 5

[Kai Engert (:KaiE:)]

Attachment: Patch v1

this is bob's patch from comment 2

r=kengert

Comment 6

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 228318 [details] [diff] [review]
Patch v1

I'm relieved to know that this patch fixes it.  I spent a lot of time going over the code looking for another source of heap corruption, without finding any.  

Please check this patch in on the trunk and NSS_3_11_BRANCH, or, if you prefer, I will do so at your request.

Comment 7

[Kai Engert (:KaiE:)]

patch checked into trunk and 3.11 branch

Checking in crmfpop.c;
/cvsroot/mozilla/security/nss/lib/crmf/crmfpop.c,v  <--  crmfpop.c
new revision: 1.7; previous revision: 1.6
done

Checking in crmfpop.c;
/cvsroot/mozilla/security/nss/lib/crmf/crmfpop.c,v  <--  crmfpop.c
new revision: 1.3.28.4; previous revision: 1.3.28.3
done

Comment 8

[Wan-Teh Chang]

The SECItem derTemp in crmf_encode_popoprivkey
may suffer from the same problem and should also
be initialized.

Comment 9

[Kai Engert (:KaiE:)]

I filed bug 346551 for comment 8.