73 lines
2.5 KiB
Diff
73 lines
2.5 KiB
Diff
From 79857f794c1d2b17d058b949fc8b30632d8c4e40 Mon Sep 17 00:00:00 2001
|
|
From: Bart Van Assche <bvanassche@acm.org>
|
|
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;
|
|
}
|
|
|