From 79857f794c1d2b17d058b949fc8b30632d8c4e40 Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Sat, 27 Apr 2019 21:12:55 -0700 Subject: [PATCH] libsnmp, ASN parsing: Avoid triggering undefined shift-left behavior A quote from https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=7199: So this is a fun corner case of asn.1 integer parsing: the C standard says that shifting a negative number left has undefined behavior. The other obvious way to do this is to use the same code on an unsigned long, and then cast to long. The result of a cast to a signed value is implementation-defined. Linus Torvalds' argument on this is that there is no sensible way for the implementation to define it other than "copy the 2's complement bits". Avoid triggering all these corner cases by using byte-copying instead of using shift operations. Fixes: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=7199 --- snmplib/asn1.c | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/snmplib/asn1.c b/snmplib/asn1.c index 333870c95..fb672c5d5 100644 --- a/snmplib/asn1.c +++ b/snmplib/asn1.c @@ -579,7 +579,11 @@ asn_parse_int(u_char * data, static const char *errpre = "parse int"; register u_char *bufp = data; u_long asn_length; - register long value = 0; + int i; + union { + long l; + unsigned char b[sizeof(long)]; + } value; if (NULL == data || NULL == datalength || NULL == type || NULL == intp) { ERROR_MSG("parse int: NULL pointer"); @@ -615,19 +619,23 @@ asn_parse_int(u_char * data, } *datalength -= (int) asn_length + (bufp - data); - if (*bufp & 0x80) - value = -1; /* integer is negative */ DEBUGDUMPSETUP("recv", data, bufp - data + asn_length); - while (asn_length--) - value = (value << 8) | *bufp++; + memset(&value.b, *bufp & 0x80 ? 0xff : 0, sizeof(value.b)); +#ifdef WORDS_BIGENDIAN + for (i = sizeof(long) - asn_length; asn_length--; i++) + value.b[i] = *bufp++; +#else + for (i = asn_length - 1; asn_length--; i--) + value.b[i] = *bufp++; +#endif - CHECK_OVERFLOW_S(value,1); + CHECK_OVERFLOW_S(value.l, 1); - DEBUGMSG(("dumpv_recv", " Integer:\t%ld (0x%.2lX)\n", value, value)); + DEBUGMSG(("dumpv_recv", " Integer:\t%ld (0x%.2lX)\n", value.l, value.l)); - *intp = value; + *intp = value.l; return bufp; }