So looking closer at Claudio's remark from this morning I found that 'e'
basically *must* be the last element in the sequence (or weird *beep*
happens). Also, 'e' never fails, since ber_link_elements doesn't have a
fail-case. So combining this information, if we end a
ber_printf_elements on 'e' and we fail we must free the pointer
ourselves.

OK?

martijn@

Index: snmp.c
===================================================================
RCS file: /cvs/src/usr.bin/snmp/snmp.c,v
retrieving revision 1.7
diff -u -p -r1.7 snmp.c
--- snmp.c      3 Oct 2019 11:02:26 -0000       1.7
+++ snmp.c      3 Oct 2019 16:32:23 -0000
@@ -252,15 +252,17 @@ fail:
 struct ber_element *
 snmp_set(struct snmp_agent *agent, struct ber_element *vblist)
 {
-       struct ber_element *pdu;
+       struct ber_element *pdu, *varbind;
 
        if ((pdu = ber_add_sequence(NULL)) == NULL)
                return NULL;
-       if (ber_printf_elements(pdu, "tddd{e", BER_CLASS_CONTEXT,
-           SNMP_C_SETREQ, arc4random() & 0x7fffffff, 0, 0, vblist) == NULL) {
+       if ((varbind = ber_printf_elements(pdu, "tddd{", BER_CLASS_CONTEXT,
+           SNMP_C_SETREQ, arc4random() & 0x7fffffff, 0, 0, vblist)) == NULL) {
                ber_free_elements(pdu);
+               ber_free_elements(vblist);
                return NULL;
        }
+       ber_link_elements(varbind, vblist);
 
        return snmp_resolve(agent, pdu, 1);
 }
@@ -428,10 +430,14 @@ snmp_package(struct snmp_agent *agent, s
                if (ber_printf_elements(message, "d{idxd}xe",
                    agent->version, msgid, UDP_MAXPACKET, &(agent->v3->level),
                    (size_t) 1, agent->v3->sec->model, securityparams,
-                   securitysize, scopedpdu) == NULL)
+                   securitysize, scopedpdu) == NULL) {
+                       ber_free_elements(scopedpdu);
                        goto fail;
-               if (ber_scanf_elements(message, "{SSe", &secparams) == -1)
+               }
+               if (ber_scanf_elements(message, "{SSe", &secparams) == -1) {
+                       ber_free_elements(secparams);
                        goto fail;
+               }
                ber_set_writecallback(secparams, snmp_v3_secparamsoffset,
                    &secparamsoffset);
                break;

Reply via email to