95458 - ASN1 decoder crashes when fed incorrect data

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P1)

Description

[Julien Pierre]

When feeding a CRL encoded in PKCS#7 format from General Dynamics to the 
CERT_DecodeCRL function, a crash occurs in the ASN1 decoder. The decoding is not 
supposed to work as x509 format is expected; however the decoder should just 
fail with an error rather than crash.

To reproduce the problem :
1)go to http://www.gd-cs.com/ieca
2) click on "download CRL"
3) select "PKCS#7"
4) save it to a file
5) run atob to convert it to binary and pipe to another file
6) use a tool that calls CERT_DecodeCRL to import the binary CRL file . I'm 
assuming crlutil will do that
7) a crash occurs with a stack looking like this :
sec_asn1d_free_child(sec_asn1d_state_struct * 0x00b79a50, int 1) line 1212 + 12 
bytes
SEC_ASN1DecoderUpdate(sec_DecoderContext_struct * 0x00b799c8, const char * 
0x00b78d8f, unsigned long 3093) line 2471 + 11 bytes
SEC_ASN1Decode(PLArenaPool * 0x00b996e0, void * 0x00b77d88, const 
sec_ASN1Template_struct * 0x002e64d0 cert_SignedCrlTemplate, const char * 
0x00b78d80, long 3108) line 2628 + 17 bytes
SEC_ASN1DecodeItem(PLArenaPool * 0x00b996e0, void * 0x00b77d88, const 
sec_ASN1Template_struct * 0x002e64d0 cert_SignedCrlTemplate, SECItemStr * 
0x0012fad8) line 2644 + 31 bytes
CERT_DecodeDERCrl(PLArenaPool * 0x00000000, SECItemStr * 0x0012fad8, int 1) line 
353 + 22 bytes

Comment 1

[Keyser Sose]

marking NEW.

Comment 2

[Wan-Teh Chang]

Did you regret joining the NSS team? ;-)

Comment 3

[Julien Pierre]

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

Comment 4

[Julien Pierre]

Attachment: Proposed patch by liqi to fix the crash in ASN.1 decoder

Comment 5

[Wan-Teh Chang]

Comment on attachment 59460 [details] [diff] [review]
Proposed patch by liqi to fix the crash in ASN.1 decoder

>-	while (state != NULL) {
>+	while (state != NULL&&stateEnd->parent!=state) {

It will be easier to read if space is added between the operators:
	while (state != NULL && stateEnd->parent != state) {

Comment 6

[Julien Pierre]

Fixed on the tip, with extra whitespace.

Checking in secasn1d.c;
/cvsroot/mozilla/security/nss/lib/util/secasn1d.c,v  <--  secasn1d.c
new revision: 1.12; previous revision: 1.11
done