From b15e289007c4cd4e273ef9ffd663d32f3d70ef27 Mon Sep 17 00:00:00 2001 From: "wtc%google.com" Date: Sat, 31 Mar 2012 04:50:22 +0000 Subject: [PATCH] Bug 715073: ASN.1 types that must have at least one content octet are not allowed to be zero length. When returning a SECItem of zero length, set the data pointer to NULL. The patch is contributed by Kaspar Brand . r=rrelyea,wtc. git-svn-id: svn://10.0.0.236/trunk@263622 18797224-902f-48f8-a5cc-f745e15eee43 --- mozilla/security/nss/lib/util/quickder.c | 69 +++++++++++++++--------- 1 file changed, 43 insertions(+), 26 deletions(-) diff --git a/mozilla/security/nss/lib/util/quickder.c b/mozilla/security/nss/lib/util/quickder.c index 3f4f20ccf84..b1956af6278 100644 --- a/mozilla/security/nss/lib/util/quickder.c +++ b/mozilla/security/nss/lib/util/quickder.c @@ -815,40 +815,57 @@ static SECStatus DecodeItem(void* dest, SECItem newtemp = temp; rv = GetItem(&newtemp, &temp, PR_FALSE); save = PR_TRUE; - if ((SECSuccess == rv) && SEC_ASN1_UNIVERSAL == (kind & SEC_ASN1_CLASS_MASK)) - switch (kind & SEC_ASN1_TAGNUM_MASK) + if ((SECSuccess == rv) && + SEC_ASN1_UNIVERSAL == (kind & SEC_ASN1_CLASS_MASK)) { - /* special cases of primitive types */ - case SEC_ASN1_INTEGER: + unsigned long tagnum = kind & SEC_ASN1_TAGNUM_MASK; + if ( temp.len == 0 && (tagnum == SEC_ASN1_BOOLEAN || + tagnum == SEC_ASN1_INTEGER || + tagnum == SEC_ASN1_BIT_STRING || + tagnum == SEC_ASN1_OBJECT_ID || + tagnum == SEC_ASN1_ENUMERATED || + tagnum == SEC_ASN1_UTC_TIME || + tagnum == SEC_ASN1_GENERALIZED_TIME) ) { - /* remove leading zeroes if the caller requested siUnsignedInteger - This is to allow RSA key operations to work */ - SECItem* destItem = (SECItem*) ((char*)dest + templateEntry->offset); - if (destItem && (siUnsignedInteger == destItem->type)) + /* these types MUST have at least one content octet */ + PORT_SetError(SEC_ERROR_BAD_DER); + rv = SECFailure; + } + else + switch (tagnum) + { + /* special cases of primitive types */ + case SEC_ASN1_INTEGER: { - while (temp.len > 1 && temp.data[0] == 0) - { /* leading 0 */ - temp.data++; - temp.len--; + /* remove leading zeroes if the caller requested + siUnsignedInteger + This is to allow RSA key operations to work */ + SECItem* destItem = (SECItem*) ((char*)dest + + templateEntry->offset); + if (destItem && (siUnsignedInteger == destItem->type)) + { + while (temp.len > 1 && temp.data[0] == 0) + { /* leading 0 */ + temp.data++; + temp.len--; + } } + break; } - break; - } - case SEC_ASN1_BIT_STRING: - { - /* change the length in the SECItem to be the number of bits */ - if (temp.len && temp.data) + case SEC_ASN1_BIT_STRING: { - temp.len = (temp.len-1)*8 - ((*(unsigned char*)temp.data) & 0x7); - temp.data = (unsigned char*)(temp.data+1); + /* change the length in the SECItem to be the number + of bits */ + temp.len = (temp.len-1)*8 - (temp.data[0] & 0x7); + temp.data += 1; + break; } - break; - } - default: - { - break; + default: + { + break; + } } } } @@ -863,7 +880,7 @@ static SECStatus DecodeItem(void* dest, If part of the destination was allocated by the decoder, in cases of POINTER, SET OF and SEQUENCE OF, then type is set to siBuffer due to the use of PORT_ArenaZAlloc*/ - destItem->data = temp.data; + destItem->data = temp.len ? temp.data : NULL; destItem->len = temp.len; } else