On Wed, Feb 20, 2002 at 02:04:46PM -0800, Guy Harris wrote:
> On Wed, Feb 20, 2002 at 09:41:16AM -0800, Breen Mullins wrote:
> > >I saw this too. I'm in the midst of rebuilding ethereal and I'll
> > >try to reproduce it.
> >
> > Verified on RedHat 7.2
>
> Spiffy.
>
> Could you send us a network trace so we can test the fix (and, if
> necessary, use it to figure out where the problem is, although
>
> > VarBind Value Type/Len: 0x04 0x84 0xffffffff (4294967295)
>
> suggests rather strongly where we'd have to add some checks).
The attached patch to "asn1.c" should keep the ASN.1 code from blowing
up if handed an absurd length for a string or OID item. (It'll throw an
exception, so that Ethereal will mark the frame with an error, but it
shouldn't crash with a problem in the memory allocator, as happened when
you tried the current version of Ethereal.)
I'll check it in (it didn't seem to break anything on some SNMP captures
I tried it on), but you should test it with your capture to make sure
Ethereal doesn't crash. (Send us the capture anyway, if it doesn't have
any proprietary data - or use Ethereal or editcap to slice out the
offending SNMP packet and just put *that* in a capture file - so that we
have a test file we can use in regression tests to make sure the bug
doesn't creep back in.)
Index: asn1.c
===================================================================
RCS file: /usr/local/cvsroot/ethereal/asn1.c,v
retrieving revision 1.8
diff -c -r1.8 asn1.c
*** asn1.c 2002/01/21 07:36:31 1.8
--- asn1.c 2002/02/20 22:32:33
***************
*** 647,652 ****
--- 647,666 ----
guchar *ptr;
eoc = asn1->offset + enc_len;
+
+ /*
+ * First, make sure the entire string is in the tvbuff, and throw
+ * an exception if it isn't. If the length is bogus, this should
+ * keep us from trying to allocate an immensely large buffer.
+ * (It won't help if the length is *valid* but immensely large,
+ * but that's another matter.)
+ *
+ * We do that by attempting to fetch the last byte (if the length
+ * isn't 0).
+ */
+ if (enc_len != 0)
+ tvb_get_guint8(asn1->tvb, eoc - 1);
+
*octets = g_malloc (enc_len);
ptr = *octets;
while (asn1->offset < eoc) {
***************
*** 795,800 ****
--- 809,828 ----
subid_t *optr;
eoc = asn1->offset + enc_len;
+
+ /*
+ * First, make sure the entire string is in the tvbuff, and throw
+ * an exception if it isn't. If the length is bogus, this should
+ * keep us from trying to allocate an immensely large buffer.
+ * (It won't help if the length is *valid* but immensely large,
+ * but that's another matter.)
+ *
+ * We do that by attempting to fetch the last byte (if the length
+ * isn't 0).
+ */
+ if (enc_len != 0)
+ tvb_get_guint8(asn1->tvb, eoc - 1);
+
size = enc_len + 1;
*oid = g_malloc(size * sizeof(gulong));
optr = *oid;
