Avoid undefined behavior when handling large ASN.1 integers

Instead of shifting bits off the top of the 32-bit value,
we mask off the top 8 bits before shifting them away, and
restore the sign bit at the end.  This still results in
a result that is not what was intended, as this code can
not handle values greater than 2^31-1 or smaller than
-2^31, but this new mechanism results in a "more correct"
garbage out, with no undefined behavior.

Author: fenner

print-snmp.c

@@ -531,18 +531,26 @@ asn1_parse(netdissect_options *ndo,
 
 			case INTEGER: {
 				uint32_t data;
+				uint32_t signbit = 0;
 				elem->type = BE_INT;
 				data = 0;
 
 				if (elem->asnlen == 0) {
 					ND_PRINT("[asnlen=0]");
 					goto invalid;
 				}
-				if (GET_U_1(p) & ASN_BIT8)	/* negative */
+				if (GET_U_1(p) & ASN_BIT8) {	/* negative */
 					data = UINT_MAX;
+					signbit = 0x80000000;
+				}
+				/* In order to avoid undefined behavior, we mask off the low
+				 * 24 bits of the data before shifting up by 8 bits.  Either
+				 * way, we lose the top 8 bits, but shifting through the sign
+				 * bit results in undefined data.  If we've shifted the sign
+				 * bit away, we will restore it below. */
 				for (i = elem->asnlen; i != 0; p++, i--)
-					data = (data << ASN_SHIFT8) | GET_U_1(p);
-				elem->data.integer = data;
+					data = ( ( data & 0x00ffffff ) << ASN_SHIFT8) | GET_U_1(p);
+				elem->data.integer = (int32_t)( data | signbit );
 				break;
 			}