On Mon, Dec 21, 2009 at 10:06 PM, Magnus Fromreide <[email protected]> wrote: > On Mon, 2009-12-21 at 12:01 +0000, [email protected] wrote: >> Revision: 17926 >> http://net-snmp.svn.sourceforge.net/net-snmp/?rev=17926&view=rev >> Author: bvassche >> Date: 2009-12-21 12:01:15 +0000 (Mon, 21 Dec 2009) >> >> Log Message: >> ----------- >> Applied patch #2912062: make sure that the string returned by >> read_config_read_octet_string() is properly terminated, such that the >> callers of this function do not trigger past-end-of-buffer reads. Found >> this issue via BoundsChecker. > > I think this patch is bogus as the result is a length + char-array, not > a zero terminated string. > > An octet string can quite legally contain embedded NUL characters and so > the bug that should be reported is that someone tries to operate on it > using a str* function. > > This is also the reason for the len argument to the function. > > I thus propose a reversal of this patch and a comment in the tracker > that explains the problem. > > To be honest I am considering a patch to never zero-terminate the result > of this function.
Modifying read_config_read_octet_string() such that it never zero-terminates its output is not as straightforward as it looks because of the assumptions made by the callers of this function. An example: the function read_config_read_data() with the first argument 'type' set to ASN_OCTET_STR calls read_config_read_octet_string() both for reading byte arrays and for reading ASCII strings. The code that implements the DISMAN MIB uses this function for reading byte arrays, and the function netsnmp_read_data_callback() uses this function for reading ASCII strings. That last function does no attempt to terminate the string that has been read itself but assumes that read_config_read_octet_string() does this. Furthermore, the function read_config_read_octet_string() terminates the string it read only if it was written in ASCII format or if a buffer overflow occurred, but not if the string was written in hex format and no buffer overflow occurred. This is inconsistent. As a consequence, if the function netsnmp_read_data_callback() is told to parse the input data "abc" as the type ASN_OCTET_STR, its output will be '\0'-terminated. If the same function is told to parse the input data 0x616263 as the type ASN_OCTET_STR, its output data will not be '\0'-terminated. Bart. ------------------------------------------------------------------------------ This SF.Net email is sponsored by the Verizon Developer Community Take advantage of Verizon's best-in-class app development support A streamlined, 14 day to market process makes app distribution fast and easy Join now and get one step closer to millions of Verizon customers http://p.sf.net/sfu/verizon-dev2dev _______________________________________________ Net-snmp-coders mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/net-snmp-coders
