DER parser: fix undefined behaviors and add missing length tests

Fix several undefined behaviors, caused by possible use or read of
uninitialized memory.
pull/3297/head
Pierre Chifflier 8 years ago committed by Victor Julien
parent d6a98aa1c3
commit 2d34e402c0

@ -147,6 +147,10 @@ static Asn1Generic * DecodeAsn1DerGeneric(const unsigned char *buffer,
*errcode = ERR_DER_RECURSION_LIMIT;
return NULL;
}
if (max_size < 2) {
*errcode = ERR_DER_INVALID_SIZE;
return NULL;
}
el.cls = (d_ptr[0] & 0xc0) >> 6;
el.pc = (d_ptr[0] & 0x20) >> 5;
@ -247,6 +251,7 @@ static Asn1Generic * DecodeAsn1DerGeneric(const unsigned char *buffer,
/* total sequence length */
const unsigned char * save_d_ptr = d_ptr;
d_ptr++;
el_max_size--;
c = d_ptr[0];
/* short form 8.1.3.4 */
@ -256,7 +261,7 @@ static Asn1Generic * DecodeAsn1DerGeneric(const unsigned char *buffer,
/* long form 8.1.3.5 */
} else {
numbytes = c & 0x7f;
if (numbytes > el_max_size) {
if (2 + numbytes > el_max_size) {
SCFree(child);
*errcode = ERR_DER_ELEMENT_SIZE_TOO_BIG;
return NULL;
@ -306,7 +311,7 @@ static Asn1Generic * DecodeAsn1DerInteger(const unsigned char *buffer,
numbytes = d_ptr[1];
if (numbytes > size) {
if ((uint32_t)(numbytes + 2) > size) {
*errcode = ERR_DER_ELEMENT_SIZE_TOO_BIG;
return NULL;
}
@ -383,6 +388,9 @@ static Asn1Generic * DecodeAsn1DerBoolean(const unsigned char *buffer,
Asn1Generic *a;
numbytes = d_ptr[1];
if ((uint32_t)(numbytes + 2) > size) {
return NULL;
}
d_ptr += 2;
if (DecodeAsn1BuildValue(&d_ptr, &value, numbytes, errcode) == -1) {
@ -410,6 +418,9 @@ static Asn1Generic * DecodeAsn1DerNull(const unsigned char *buffer,
Asn1Generic *a;
numbytes = d_ptr[1];
if ((uint32_t)(numbytes + 2) > size) {
return NULL;
}
d_ptr += 2;
if (DecodeAsn1BuildValue(&d_ptr, &value, numbytes, errcode) == -1) {
@ -447,13 +458,16 @@ static Asn1Generic * DecodeAsn1DerBitstring(const unsigned char *buffer,
/* long form 8.1.3.5 */
} else {
numbytes = c & 0x7f;
if ((uint32_t)(numbytes + 2) > max_size) {
return NULL;
}
d_ptr++;
if (DecodeAsn1BuildValue(&d_ptr, &length, numbytes, errcode) == -1) {
return NULL;
}
}
if (length > max_size)
if ((d_ptr-buffer) + length > max_size)
return NULL;
a = Asn1GenericNew();
@ -497,13 +511,16 @@ static Asn1Generic * DecodeAsn1DerOid(const unsigned char *buffer,
/* long form 8.1.3.5 */
} else {
numbytes = c & 0x7f;
if ((uint32_t)(numbytes + 2) > max_size) {
return NULL;
}
d_ptr++;
if (DecodeAsn1BuildValue(&d_ptr, &oid_length, numbytes, errcode) == -1) {
return NULL;
}
}
if (oid_length > max_size)
if (oid_length == 0 || (d_ptr-buffer) + oid_length > max_size)
return NULL;
a = Asn1GenericNew();
@ -522,6 +539,12 @@ static Asn1Generic * DecodeAsn1DerOid(const unsigned char *buffer,
snprintf(a->str, MAX_OID_LENGTH, "%d.%d", (d_ptr[0]/40), (d_ptr[0]%40));
d_ptr++;
if (oid_length + (d_ptr-buffer) > max_size) {
SCFree(a->str);
SCFree(a);
return NULL;
}
/* sub-identifiers are multi-valued, coded and 7 bits, first bit of
the 8bits is used to indicate, if a new value is starting */
for (i=1; i<oid_length; ) {
@ -554,6 +577,7 @@ static Asn1Generic * DecodeAsn1DerIA5String(const unsigned char *buffer,
unsigned char c;
d_ptr++;
max_size--;
/* total sequence length */
c = d_ptr[0];
@ -561,16 +585,21 @@ static Asn1Generic * DecodeAsn1DerIA5String(const unsigned char *buffer,
if ((c & (1<<7))>>7 == 0) {
length = c;
d_ptr++;
max_size--;
/* long form 8.1.3.5 */
} else {
numbytes = c & 0x7f;
if (max_size < 1 + numbytes) {
return NULL;
}
max_size -= 1 + numbytes;
d_ptr++;
if (DecodeAsn1BuildValue(&d_ptr, &length, numbytes, errcode) == -1) {
return NULL;
}
}
if (length == UINT32_MAX || length > max_size) {
if (length == UINT32_MAX || (d_ptr-buffer) + length > max_size) {
*errcode = ERR_DER_ELEMENT_SIZE_TOO_BIG;
return NULL;
}
@ -606,6 +635,7 @@ static Asn1Generic * DecodeAsn1DerOctetString(const unsigned char *buffer,
unsigned char c;
d_ptr++;
max_size--;
/* total sequence length */
c = d_ptr[0];
@ -613,20 +643,24 @@ static Asn1Generic * DecodeAsn1DerOctetString(const unsigned char *buffer,
if ((c & (1<<7))>>7 == 0) {
length = c;
d_ptr++;
max_size--;
/* long form 8.1.3.5 */
} else {
numbytes = c & 0x7f;
if (max_size < 1 + numbytes) {
return NULL;
}
max_size -= 1 + numbytes;
d_ptr++;
if (DecodeAsn1BuildValue(&d_ptr, &length, numbytes, errcode) == -1) {
return NULL;
}
}
if (length == UINT32_MAX || length > max_size) {
if (length == UINT32_MAX || (d_ptr-buffer) + length > max_size) {
*errcode = ERR_DER_ELEMENT_SIZE_TOO_BIG;
return NULL;
}
a = Asn1GenericNew();
if (a == NULL)
return NULL;
@ -683,12 +717,15 @@ static Asn1Generic * DecodeAsn1DerPrintableString(const unsigned char *buffer,
} else {
numbytes = c & 0x7f;
d_ptr++;
if (2 + numbytes > max_size) {
return NULL;
}
if (DecodeAsn1BuildValue(&d_ptr, &length, numbytes, errcode) == -1) {
return NULL;
}
}
if (length == UINT32_MAX || length > max_size) {
if (length == UINT32_MAX || (d_ptr-buffer) + length > max_size) {
*errcode = ERR_DER_ELEMENT_SIZE_TOO_BIG;
return NULL;
}
@ -742,6 +779,10 @@ static Asn1Generic * DecodeAsn1DerSequence(const unsigned char *buffer,
} else {
numbytes = c & 0x7f;
d_ptr++;
if ( 2 + numbytes > max_size ) {
SCFree(node);
return NULL;
}
if (DecodeAsn1BuildValue(&d_ptr, &d_length, numbytes, errcode) == -1) {
SCFree(node);
return NULL;
@ -813,6 +854,10 @@ static Asn1Generic * DecodeAsn1DerSet(const unsigned char *buffer,
/* long form 8.1.3.5 */
} else {
numbytes = c & 0x7f;
if (2 + numbytes > max_size) {
SCFree(node);
return NULL;
}
d_ptr++;
if (DecodeAsn1BuildValue(&d_ptr, &d_length, numbytes, errcode) == -1) {
SCFree(node);
@ -909,6 +954,9 @@ Asn1Generic * DecodeDer(const unsigned char *buffer, uint32_t size,
numbytes = c & 0x7f;
d_ptr += 2;
if (size < numbytes + 2) {
return NULL;
}
if (DecodeAsn1BuildValue(&d_ptr, &d_length, numbytes, errcode) == -1) {
return NULL;
}

Loading…
Cancel
Save