[ net-snmp-Bugs-2911569 ] crash in read_config.c

[SourceForge.net <no...@so...>]

Bugs item #2911569, was opened at 2009-12-09 13:15
Message generated for change (Comment added) made by tomatoyohe
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=112694&aid=2911569&group_id=12694

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: agent
Group: None
>Status: Closed
>Resolution: Fixed
Priority: 5
Private: No
Submitted By: Thomas Yohe (tomatoyohe)
Assigned to: Nobody/Anonymous (nobody)
Summary: crash in read_config.c

Initial Comment:
in net-snmp 5.5 and 5.4.2.1, Microsoft Application Verifier finds a case where NULL being passed to strelen when evaluating the following:
        if (strncmp(cptr2, perspath, strlen(perspath)) == 0 ||
            (persfile != NULL &&
             strncmp(cptr2, persfile, strlen(persfile)) == 0)) {
in the above persfile==NULL.  although the author checks for NULL, however the strcmp() still takes place.  This needs to be broken up into a less complex if.
I tried to add screen shots of ApplicationVerifier, but was limited to a 256K artifact upload.

----------------------------------------------------------------------

>Comment By: Thomas Yohe (tomatoyohe)
Date: 2009-12-21 11:08

Message:
I can confirm that patches  r17928 + r17930 to read_config.c appear to fix
this issue.   Thank you.  

----------------------------------------------------------------------

Comment By: Thomas Yohe (tomatoyohe)
Date: 2009-12-21 11:08

Message:
Thanks for the bug report!
We've fixed the problem in the 5.4.x code branch
and the main development tree, so it should be
fixed in future releases of the Net-SNMP package.

----------------------------------------------------------------------

Comment By: Bart Van Assche (bvassche)
Date: 2009-12-21 09:08

Message:
Can you please verify whether the two patches r17928 + r17930 fix this
issue ? These two patches have been checked in on the 5.3, 5.4 and 5.5
branches and also on the trunk.

----------------------------------------------------------------------

Comment By: Thomas Yohe (tomatoyohe)
Date: 2009-12-09 13:39

Message:
This is the code that is freeing up what perspath was pointing to:
    /*
     * is some silly person is calling us with our own pointer?
     */
    if (netsnmp_ds_strings[storeid][which] == value)
        return SNMPERR_SUCCESS;
    
    if (netsnmp_ds_strings[storeid][which] != NULL) {
        free(netsnmp_ds_strings[storeid][which]);
	netsnmp_ds_strings[storeid][which] = NULL;
    }


----------------------------------------------------------------------

Comment By: Thomas Yohe (tomatoyohe)
Date: 2009-12-09 13:35

Message:
The true root cause of this problem is that, "perspath" is pointing to a
string in the netsnmp_ds_strings[storeid][which], however in the course of
processing, this string gets freed down in the bowels of default_store.c. 
But the caller does not know that the rug has been pulled out and continues
to reference perspath as if it still pointed to valid memory.  ultimately
this leads to heap corruption.

----------------------------------------------------------------------

Comment By: Thomas Yohe (tomatoyohe)
Date: 2009-12-09 13:18

Message:
actually in the above, the true problem is that perspath (not persfile) is
set to a value that has been freed.  More to follow on this.

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=112694&aid=2911569&group_id=12694