#1265 snmp client code crashes on bad InetAddress6

Bill Fenner

When you have an InetAddressType.InetAddress that's, say, 2.16.4294967200.4294967200.4294967200.4294967200.4294967200.4294967200.4294967200.4294967200.4294967200.4294967200.4294967200.4294967200.4294967200.4294967200.4294967200.4294967200, because the MIB implementation accidentally sign-extends bytes when it copies addresses, we overflow a stack-allocated buffer in mib.c and crash.

I added range checking to both IPv4 and IPv6. (A plain IPv4 address can not overflow the buffer, but an IPv4z can.) If we are outside the range, we fall back to "not handled".

Unfortunately, the "not handled" code then prints the buffer like an ASCII string, which is also wrong since there are values >255 in the oid string, but, one problem at a time. Ugly is better than crash. :-)

1 Attachments


  • Bill Fenner

    Bill Fenner - 2013-10-06

    Why would you want to return an IPv4 address formatted like 4294967200.7.0.1 or an IPv6 address formatted like ffffffa0:10:ffffff27:... ?

    When the IP address parser is clearly not parsing an IP address, it makes more sense to me to return "I am not handling this OID" rather than to try to handle it and truncate it.

  • bart

    bart - 2013-10-07

    How about adding the patch below on top of the previous patch ? I think that implements your idea of refusing invalid addresses but with less code.

    diff --git a/snmplib/mib.c b/snmplib/mib.c
    index fb56846..e820c52 100644
    --- a/snmplib/mib.c
    +++ b/snmplib/mib.c
    @@ -4123,6 +4123,10 @@ dump_realloc_oid_to_inetaddress(const int addr_type, const oid * objid, size_t o
         if (!buf)
             return 1;
    +    for (i = 0; i < objidlen; i++)
    +        if (objid[i] < 0 || objid[i] > 255)
    +            return 2;
         p = intbuf;
         *p++ = quotechar;
    • Bill Fenner

      Bill Fenner - 2013-10-07


      I noticed an existing bug: the *Z handlers will probably not output the right zone ID on 64-bit platforms. Now that there's a test suite it'll be easy to expose this :-)

      I also noticed your test suite only used -1 and not, e.g., 256 for invalid values. There should probably be at least one test for that.

  • bart

    bart - 2013-10-19

    The attached patch including the modification mentioned above and a test case for 256 has been committed on all active branches (v5.4 and up). Thanks for reporting this.

  • bart

    bart - 2013-10-19
    • status: open --> closed

Log in to post a comment.

Get latest updates about Open Source Projects, Conferences and News.

Sign up for the SourceForge newsletter:

No, thanks