552946 - cmmf_create_witness_and_challenge didn't handle OOM from SEC_ASN1EncodeInteger

Status: RESOLVED WONTFIX

(NSS :: Libraries, defect, P5)

Description

[timeless]

49 cmmf_create_witness_and_challenge(PRArenaPool     *poolp,

69     encodedRandNum = SEC_ASN1EncodeInteger(poolp, &challenge->randomNumber,
70                                            inRandom);
> 1636 SEC_ASN1EncodeInteger(PRArenaPool *poolp, SECItem *dest, long value)
>> 1589 sec_asn1e_integer(PRArenaPool *poolp, SECItem *dest, unsigned long value,
>> 1617     dest = sec_asn1e_allocate_item (poolp, dest, len);
>>> 1514 sec_asn1e_allocate_item (PRArenaPool *poolp, SECItem *dest, unsigned long len)
>>> 1522         if (dest != NULL) {
>>> 1523             dest->data = (unsigned char*)PORT_ArenaAlloc (poolp, len);
>>> 1524             if (dest->data == NULL) {
>>> 1525                 dest = NULL;
>>> 1528         if (dest == NULL) {
>>> 1530             PORT_ArenaRelease (poolp, release);
>>> 1552     return dest;
>> 1618     if (dest == NULL)
>> 1619         return NULL;
> 1638     return sec_asn1e_integer (poolp, dest, (unsigned long) value, PR_FALSE);

encodedRandNum is NULL because 1523 failed and we cleaned things up.

We now stomp on our null OOM marking pointer with something which was freed by some pool magic:
71     encodedRandNum = &challenge->randomNumber;

Unhappiness follows:

77     rv = PK11_HashBuf(SEC_OID_SHA1, randHash, encodedRandNum->data, 
78                       (PRUint32)encodedRandNum->len);

Comment 1

[Nelson Bolyard (seldom reads bugmail)]

I'm not certain, but I think the cmmf code is all dead. 
I don't think FF or TB use it, and I'm sure nothing else does,
except for an NSS test program. 
Given the diminishing nature of NSS development resources, 
I'd prefer to remove/eliminate dead code than to fix it.

Comment 2

[Shailen]

Attachment: Patch V 1

Hi Nelson,

    Can you please review this patch ?

Thanks,
Shailendra

Comment 3

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 434218 [details] [diff] [review]
Patch V 1

Doesn't fix the problem.