On Tue, Mar 10, 2020 at 01:13:46PM +0100, Martijn van Duren wrote:
> On 3/10/20 10:21 AM, Jan Klemkow wrote:
> > On Tue, Mar 10, 2020 at 12:27:20AM +0100, Martijn van Duren wrote:
> >> Looking at RFC1157 section 4.1.6, an snmpv1 trap should also contain a
> >> varbindlist.
> >>
> >> Could you test the diff below?
> > 
> > Is also OK for me and the current call path seems to be clean.
> > But, shouldn't we set iter to NULL anyway?
> 
> If we state that traphandler_parse must initialize *vbinds, then not
> setting it allows us to differentiate between undefined behaviour
> (initialized to garbage) and no varbindlist. The first one will only
> show its head if someone screws up a refactor.
> But if you feel strongly about it we can initialize it.

I don't feel strong about it.  Commit your change and I fine with it.

OK jan@

> >> On 3/9/20 11:38 PM, Jan Klemkow wrote:
> >>> Hi,
> >>>
> >>> The following diff fixes the use of the uninitialized pointer iter
> >>> in trapcmd_exec().
> >>>
> >>> iter is just initialized in traphandler_parse() if vers is SNMP_V2.  In
> >>> all other cases iter stays uninitialized and may dereferenced in
> >>> trapcmd_exec().
> >>>
> >>> OK?
> >>>
> >>> bye,
> >>> Jan
> >>>
> >>
> >> Index: traphandler.c
> >> ===================================================================
> >> RCS file: /cvs/src/usr.sbin/snmpd/traphandler.c,v
> >> retrieving revision 1.15
> >> diff -u -p -r1.15 traphandler.c
> >> --- traphandler.c  24 Oct 2019 12:39:27 -0000      1.15
> >> +++ traphandler.c  9 Mar 2020 23:26:56 -0000
> >> @@ -236,6 +236,8 @@ traphandler_parse(char *buf, size_t n, s
> >>                trapoid, &gtype, &etype, uptime) == -1)
> >>                    goto done;
> >>            traphandler_v1translate(trapoid, gtype, etype);
> >> +          *vbinds =
> >> +              elm->be_sub->be_next->be_next->be_next->be_next->be_next;
> >>            break;
> >>  
> >>    case SNMP_V2:
> >>
> 

Reply via email to