Re: snmpd(8): Log correct engineid
ok Martijn van Duren(openbsd+t...@list.imperialat.at) on 2021.10.21 08:45:51 +0100: > ping > > On Sun, 2021-09-26 at 10:22 +0200, Martijn van Duren wrote: > > ober_get_nstring writes a pointer to buf and does not overwrite the > > content of buf itself. So pushing an array in there will result in it > > only writing the pointer address to the array, which is not exactly what > > we want to show. > > > > I choose to go for sizeof, instead of using the define to be a little > > more on the safe side, but I didn't change SNMPD_MAXCONTEXNAMELEN to > > keep the diff small. > > > > OK? > > > > martijn@ > > > > Index: snmpe.c > > === > > RCS file: /cvs/src/usr.sbin/snmpd/snmpe.c,v > > retrieving revision 1.76 > > diff -u -p -r1.76 snmpe.c > > --- snmpe.c 6 Sep 2021 13:32:18 - 1.76 > > +++ snmpe.c 26 Sep 2021 08:19:59 - > > @@ -237,7 +237,7 @@ snmpe_parse(struct snmp_message *msg) > > long longerrval, erridx; > > u_intclass; > > char*comn; > > - char*flagstr, *ctxname; > > + char*flagstr, *ctxname, *engineid; > > size_t len; > > struct sockaddr_storage *ss = &msg->sm_ss; > > struct ber_element *root = msg->sm_req; > > @@ -300,9 +300,12 @@ snmpe_parse(struct snmp_message *msg) > > } > > > > if (ober_scanf_elements(a, "{xxeS$}$", > > - &msg->sm_ctxengineid, &msg->sm_ctxengineid_len, > > - &ctxname, &len, &msg->sm_pdu) != 0) > > + &engineid, &msg->sm_ctxengineid_len, &ctxname, &len, > > + &msg->sm_pdu) != 0) > > goto parsefail; > > + if (msg->sm_ctxengineid_len > sizeof(msg->sm_ctxengineid)) > > + goto parsefail; > > + memcpy(msg->sm_ctxengineid, engineid, msg->sm_ctxengineid_len); > > if (len > SNMPD_MAXCONTEXNAMELEN) > > goto parsefail; > > memcpy(msg->sm_ctxname, ctxname, len); > > > >
Re: snmpd(8): Log correct engineid
ping On Sun, 2021-09-26 at 10:22 +0200, Martijn van Duren wrote: > ober_get_nstring writes a pointer to buf and does not overwrite the > content of buf itself. So pushing an array in there will result in it > only writing the pointer address to the array, which is not exactly what > we want to show. > > I choose to go for sizeof, instead of using the define to be a little > more on the safe side, but I didn't change SNMPD_MAXCONTEXNAMELEN to > keep the diff small. > > OK? > > martijn@ > > Index: snmpe.c > === > RCS file: /cvs/src/usr.sbin/snmpd/snmpe.c,v > retrieving revision 1.76 > diff -u -p -r1.76 snmpe.c > --- snmpe.c 6 Sep 2021 13:32:18 - 1.76 > +++ snmpe.c 26 Sep 2021 08:19:59 - > @@ -237,7 +237,7 @@ snmpe_parse(struct snmp_message *msg) > long longerrval, erridx; > u_intclass; > char*comn; > - char*flagstr, *ctxname; > + char*flagstr, *ctxname, *engineid; > size_t len; > struct sockaddr_storage *ss = &msg->sm_ss; > struct ber_element *root = msg->sm_req; > @@ -300,9 +300,12 @@ snmpe_parse(struct snmp_message *msg) > } > > if (ober_scanf_elements(a, "{xxeS$}$", > - &msg->sm_ctxengineid, &msg->sm_ctxengineid_len, > - &ctxname, &len, &msg->sm_pdu) != 0) > + &engineid, &msg->sm_ctxengineid_len, &ctxname, &len, > + &msg->sm_pdu) != 0) > goto parsefail; > + if (msg->sm_ctxengineid_len > sizeof(msg->sm_ctxengineid)) > + goto parsefail; > + memcpy(msg->sm_ctxengineid, engineid, msg->sm_ctxengineid_len); > if (len > SNMPD_MAXCONTEXNAMELEN) > goto parsefail; > memcpy(msg->sm_ctxname, ctxname, len); >
snmpd(8): Log correct engineid
ober_get_nstring writes a pointer to buf and does not overwrite the content of buf itself. So pushing an array in there will result in it only writing the pointer address to the array, which is not exactly what we want to show. I choose to go for sizeof, instead of using the define to be a little more on the safe side, but I didn't change SNMPD_MAXCONTEXNAMELEN to keep the diff small. OK? martijn@ Index: snmpe.c === RCS file: /cvs/src/usr.sbin/snmpd/snmpe.c,v retrieving revision 1.76 diff -u -p -r1.76 snmpe.c --- snmpe.c 6 Sep 2021 13:32:18 - 1.76 +++ snmpe.c 26 Sep 2021 08:19:59 - @@ -237,7 +237,7 @@ snmpe_parse(struct snmp_message *msg) long longerrval, erridx; u_intclass; char*comn; - char*flagstr, *ctxname; + char*flagstr, *ctxname, *engineid; size_t len; struct sockaddr_storage *ss = &msg->sm_ss; struct ber_element *root = msg->sm_req; @@ -300,9 +300,12 @@ snmpe_parse(struct snmp_message *msg) } if (ober_scanf_elements(a, "{xxeS$}$", - &msg->sm_ctxengineid, &msg->sm_ctxengineid_len, - &ctxname, &len, &msg->sm_pdu) != 0) + &engineid, &msg->sm_ctxengineid_len, &ctxname, &len, + &msg->sm_pdu) != 0) goto parsefail; + if (msg->sm_ctxengineid_len > sizeof(msg->sm_ctxengineid)) + goto parsefail; + memcpy(msg->sm_ctxengineid, engineid, msg->sm_ctxengineid_len); if (len > SNMPD_MAXCONTEXNAMELEN) goto parsefail; memcpy(msg->sm_ctxname, ctxname, len);