Bug 55303 - Hex output of ucd-snmp library broken by security patch
Hex output of ucd-snmp library broken by security patch
Status: CLOSED RAWHIDE
Product: Red Hat Linux
Classification: Retired
Component: ucd-snmp (Show other bugs)
7.2
All Linux
high Severity high
: ---
: ---
Assigned To: Phil Knirsch
Brock Organ
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2001-10-29 10:39 EST by Need Real Name
Modified: 2015-03-04 20:09 EST (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2001-12-11 20:52:52 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description Need Real Name 2001-10-29 10:39:37 EST
From Bugzilla Helper:
User-Agent: Mozilla/4.0 (compatible; MSIE 6.0; Windows NT 5.0)

Description of problem:
The ucd-snmp-4.2.1-security2.patch patch for the ucd-snmp-4.2.1 library 
introduces some bugs! The first bug is a missing "cp+=8" after the patch. 
This creates wrong output on all hex output if the output is longer than 8 
bytes.

The second problem is that the patch is not thread safe at all. The binit
() function is used with NULL as the first argument and then returns an 
static buffer. The number of buffers is hardcoded to 16. This could be a 
problem. The other problem is that the argument  "n  = (n + 1) & 15" is 
not thread safe. Therefore in a multithread environment the snmp library 
will print false results.

CU
Marius

Source ucd-snmp-4.2.1/snmplib/mib.c function binit()
struct sbuf *binit(struct sbuf *bp, char *buf, unsigned int size)
{
        static struct sbuf      temp[16];
        static unsigned int     n;

        if (size == 0) {
                if (bp)
                        memset(bp, 0, sizeof(*bp));
                return NULL;
        }
        if (bp == NULL) {
                bp = temp + n;
                n  = (n + 1) & 15;
        }
        bp->base = buf;
        bp->ptr  = buf;
        bp->end  = buf + size;
        bp->base[0] = '\0';
        return bp;
}



Original Source: in ucd-snmp-4.2.1/snmplib/mib.c function print_hexstring()
    for(; len >= 16; len -= 16){
        sprintf(buf, "%02X %02X %02X %02X %02X %02X %02X %02X ", cp[0], cp
[1], cp[2], cp[3], cp[4], cp[5], cp[6], cp[7]);
        buf += strlen(buf);
        cp += 8;
        sprintf(buf, "%02X %02X %02X %02X %02X %02X %02X %02X", cp[0], cp
[1], cp[2], cp[3], cp[4], cp[5], cp[6], cp[7]);
        buf += strlen(buf);
        cp += 8;
        if (ds_get_boolean(DS_LIBRARY_ID, DS_LIB_PRINT_HEX_TEXT))
        {
                sprintf(buf, "  [");
                buf += strlen(buf);
                for (tp = cp - 16; tp < cp; tp ++)
                {
                        sprint_char(buf++, *tp);
                }
                sprintf(buf, "]");
                buf += strlen(buf);
        }
        if (len > 16) { *buf++ = '\n'; *buf = 0; }
    }

Patched Source with ucd-snmp-4.2.1-security2.patch: in ucd-snmp-
4.2.1/snmplib/mib.c function print_hexstring()
    for(; len >= 16; len -= 16){
        bprintf(buf, "%02X %02X %02X %02X %02X %02X %02X %02X ", cp[0], cp
[1], cp[2], cp[3], cp[4], cp[5], cp[6], cp[7]);
        cp += 8;
        bprintf(buf, "%02X %02X %02X %02X %02X %02X %02X %02X", cp[0], cp
[1], cp[2], cp[3], cp[4], cp[5], cp[6], cp[7]);
        if (ds_get_boolean(DS_LIBRARY_ID, DS_LIB_PRINT_HEX_TEXT))
        {
                bprintf(buf, "  [");
                for (tp = cp - 16; tp < cp; tp ++)
                {
                        bputc(buf, *tp);
                }
                bprintf(buf, "]");
        }
        if (len > 16) bputc(buf, '\n');
    }


Version-Release number of selected component (if applicable):


How reproducible:
Always

Steps to Reproduce:
1. Use installed library and read out MIB value with hex output and length 
of over 8 bytes.


	

Actual Results:  Wrong output,

Additional info:

The bug is in the security update of RedHat to the original ucd-snmp 
source!
Comment 1 Need Real Name 2001-11-06 15:54:07 EST
This bug is also included in the security updated ucd-snmp packages for RedHat 
7.1 and older versions!

After installing the upgrade the same problem exists there.
Comment 2 Gerald Combs 2001-12-11 20:50:56 EST
FWIW, Ethereal uses the routines sprint_value and sprint_objid from UCD/NetSNMP.
 This patch changes the definition of each function thusly:

-char * sprint_objid(char *buf, oid *objid, size_t objidlen)
+char * sprint_objid(struct sbuf *buf, oid *objid, size_t objidlen)

-void sprint_value (char *, oid *, size_t, struct variable_list *);
+void sprint_value (struct sbuf *, oid *, size_t, struct variable_list *);

Since Ethereal always passes in a char * as the first argument, it segfaults
with the new library.
Comment 3 Phil Knirsch 2002-01-29 09:52:56 EST
The latest version (4.2.3) is available via rawhide now. This should fix this
problem as the huge patch has been removed as the buffer overflows have been
fixed upstream for good.

Thanks,

Read ya, Phil

Note You need to log in before you can comment on or make changes to this bug.