KEYS: Fix ASN.1 indefinite length object parsing
This fixes CVE-2016-0758. In the ASN.1 decoder, when the length field of an ASN.1 value is extracted, it isn't validated against the remaining amount of data before being added to the cursor. With a sufficiently large size indicated, the check: datalen - dp < 2 may then fail due to integer overflow. Fix this by checking the length indicated against the amount of remaining data in both places a definite length is determined. Whilst we're at it, make the following changes: (1) Check the maximum size of extended length does not exceed the capacity of the variable it's being stored in (len) rather than the type that variable is assumed to be (size_t). (2) Compare the EOC tag to the symbolic constant ASN1_EOC rather than the integer 0. (3) To reduce confusion, move the initialisation of len outside of: for (len = 0; n > 0; n--) { since it doesn't have anything to do with the loop counter n. Signed-off-by: David Howells <dhowells@redhat.com> Reviewed-by: Mimi Zohar <zohar@linux.vnet.ibm.com> Acked-by: David Woodhouse <David.Woodhouse@intel.com> Acked-by: Peter Jones <pjones@redhat.com>
This commit is contained in:
parent
685764b108
commit
23c8a812dc
|
@ -74,7 +74,7 @@ next_tag:
|
||||||
|
|
||||||
/* Extract a tag from the data */
|
/* Extract a tag from the data */
|
||||||
tag = data[dp++];
|
tag = data[dp++];
|
||||||
if (tag == 0) {
|
if (tag == ASN1_EOC) {
|
||||||
/* It appears to be an EOC. */
|
/* It appears to be an EOC. */
|
||||||
if (data[dp++] != 0)
|
if (data[dp++] != 0)
|
||||||
goto invalid_eoc;
|
goto invalid_eoc;
|
||||||
|
@ -96,10 +96,8 @@ next_tag:
|
||||||
|
|
||||||
/* Extract the length */
|
/* Extract the length */
|
||||||
len = data[dp++];
|
len = data[dp++];
|
||||||
if (len <= 0x7f) {
|
if (len <= 0x7f)
|
||||||
dp += len;
|
goto check_length;
|
||||||
goto next_tag;
|
|
||||||
}
|
|
||||||
|
|
||||||
if (unlikely(len == ASN1_INDEFINITE_LENGTH)) {
|
if (unlikely(len == ASN1_INDEFINITE_LENGTH)) {
|
||||||
/* Indefinite length */
|
/* Indefinite length */
|
||||||
|
@ -110,14 +108,18 @@ next_tag:
|
||||||
}
|
}
|
||||||
|
|
||||||
n = len - 0x80;
|
n = len - 0x80;
|
||||||
if (unlikely(n > sizeof(size_t) - 1))
|
if (unlikely(n > sizeof(len) - 1))
|
||||||
goto length_too_long;
|
goto length_too_long;
|
||||||
if (unlikely(n > datalen - dp))
|
if (unlikely(n > datalen - dp))
|
||||||
goto data_overrun_error;
|
goto data_overrun_error;
|
||||||
for (len = 0; n > 0; n--) {
|
len = 0;
|
||||||
|
for (; n > 0; n--) {
|
||||||
len <<= 8;
|
len <<= 8;
|
||||||
len |= data[dp++];
|
len |= data[dp++];
|
||||||
}
|
}
|
||||||
|
check_length:
|
||||||
|
if (len > datalen - dp)
|
||||||
|
goto data_overrun_error;
|
||||||
dp += len;
|
dp += len;
|
||||||
goto next_tag;
|
goto next_tag;
|
||||||
|
|
||||||
|
|
Loading…
Reference in New Issue