Plug some memory leaks.

For ARCNET and MAC addresses, don't convert them to binary until we get
to gen_acode() and gen_ecode(); instead, just save the string in a buffe
that's allocated in a way that gets cleaned up when the parser finishes,
the same way we do for some other string tokens.  Otherwise, if the
parser fails before we get to free it, it gets leaked; that was
happening.

Save the generated binary address in the parser state until we're done
with it, so that, if a call that uses the parser state calls
bpf_error(), the generated binary address gets freed.

Credit to OSS-Fuzz for finding this issue.

diff --git a/gencode.c b/gencode.c
index 1b20124b40309fe2453b9ac051d4676082e315d6..7aa9638746d8dc4e603798756213fe714b4df2ab 100644 (file)
--- a/gencode.c
+++ b/gencode.c
@@ -278,6 +278,13 @@ struct _compiler_state {
         */
        struct addrinfo *ai;
 
+       /*
+        * Another thing that's allocated is the result of pcap_ether_aton();
+        * it must be freed with free().  This variable points to any
+        * address that would need to be freed.
+        */
+       u_char *e;
+
        /*
         * Various code constructs need to know the layout of the packet.
         * These values give the necessary offsets from the beginning
@@ -710,6 +717,7 @@ pcap_compile(pcap_t *p, struct bpf_program *program,
 #ifdef INET6
        cstate.ai = NULL;
 #endif
+       cstate.e = NULL;
        cstate.ic.root = NULL;
        cstate.ic.cur_mark = 0;
        cstate.bpf_pcap = p;
@@ -720,6 +728,8 @@ pcap_compile(pcap_t *p, struct bpf_program *program,
                if (cstate.ai != NULL)
                        freeaddrinfo(cstate.ai);
 #endif
+               if (cstate.e != NULL)
+                       free(cstate.e);
                rc = -1;
                goto quit;
        }
@@ -6916,36 +6926,49 @@ gen_mcode6(compiler_state_t *cstate, const char *s1, const char *s2,
 #endif /*INET6*/
 
 struct block *
-gen_ecode(compiler_state_t *cstate, const u_char *eaddr, struct qual q)
+gen_ecode(compiler_state_t *cstate, const char *s, struct qual q)
 {
        struct block *b, *tmp;
 
        if ((q.addr == Q_HOST || q.addr == Q_DEFAULT) && q.proto == Q_LINK) {
+               cstate->e = pcap_ether_aton(s);
+               if (cstate->e == NULL)
+                       bpf_error(cstate, "malloc");
                switch (cstate->linktype) {
                case DLT_EN10MB:
                case DLT_NETANALYZER:
                case DLT_NETANALYZER_TRANSPARENT:
                        tmp = gen_prevlinkhdr_check(cstate);
-                       b = gen_ehostop(cstate, eaddr, (int)q.dir);
+                       b = gen_ehostop(cstate, cstate->e, (int)q.dir);
                        if (tmp != NULL)
                                gen_and(tmp, b);
-                       return b;
+                       break;
                case DLT_FDDI:
-                       return gen_fhostop(cstate, eaddr, (int)q.dir);
+                       b = gen_fhostop(cstate, cstate->e, (int)q.dir);
+                       break;
                case DLT_IEEE802:
-                       return gen_thostop(cstate, eaddr, (int)q.dir);
+                       b = gen_thostop(cstate, cstate->e, (int)q.dir);
+                       break;
                case DLT_IEEE802_11:
                case DLT_PRISM_HEADER:
                case DLT_IEEE802_11_RADIO_AVS:
                case DLT_IEEE802_11_RADIO:
                case DLT_PPI:
-                       return gen_wlanhostop(cstate, eaddr, (int)q.dir);
+                       b = gen_wlanhostop(cstate, cstate->e, (int)q.dir);
+                       break;
                case DLT_IP_OVER_FC:
-                       return gen_ipfchostop(cstate, eaddr, (int)q.dir);
+                       b = gen_ipfchostop(cstate, cstate->e, (int)q.dir);
+                       break;
                default:
+                       free(cstate->e);
+                       cstate->e = NULL;
                        bpf_error(cstate, "ethernet addresses supported only on ethernet/FDDI/token ring/802.11/ATM LANE/Fibre Channel");
+                       /* NOTREACHED */
                        break;
                }
+               free(cstate->e);
+               cstate->e = NULL;
+               return (b);
        }
        bpf_error(cstate, "ethernet address used in non-ether expression");
        /* NOTREACHED */
@@ -8127,16 +8150,24 @@ gen_p80211_fcdir(compiler_state_t *cstate, int fcdir)
 }
 
 struct block *
-gen_acode(compiler_state_t *cstate, const u_char *eaddr, struct qual q)
+gen_acode(compiler_state_t *cstate, const char *s, struct qual q)
 {
+       struct block *b;
+
        switch (cstate->linktype) {
 
        case DLT_ARCNET:
        case DLT_ARCNET_LINUX:
                if ((q.addr == Q_HOST || q.addr == Q_DEFAULT) &&
-                   q.proto == Q_LINK)
-                       return (gen_ahostop(cstate, eaddr, (int)q.dir));
-               else {
+                   q.proto == Q_LINK) {
+                       cstate->e = pcap_ether_aton(s);
+                       if (cstate->e == NULL)
+                               bpf_error(cstate, "malloc");
+                       b = gen_ahostop(cstate, cstate->e, (int)q.dir);
+                       free(cstate->e);
+                       cstate->e = NULL;
+                       return (b);
+               } else {
                        bpf_error(cstate, "ARCnet address used in non-arc expression");
                        /* NOTREACHED */
                }

diff --git a/gencode.h b/gencode.h
index 6a6fda587160b495360c6f910cf566a0d525aa0a..e97e90fee4e19fab221a3253211fae53ef7879a2 100644 (file)
--- a/gencode.h
+++ b/gencode.h
@@ -297,8 +297,8 @@ void gen_or(struct block *, struct block *);
 void gen_not(struct block *);
 
 struct block *gen_scode(compiler_state_t *, const char *, struct qual);
-struct block *gen_ecode(compiler_state_t *, const u_char *, struct qual);
-struct block *gen_acode(compiler_state_t *, const u_char *, struct qual);
+struct block *gen_ecode(compiler_state_t *, const char *, struct qual);
+struct block *gen_acode(compiler_state_t *, const char *, struct qual);
 struct block *gen_mcode(compiler_state_t *, const char *, const char *,
     unsigned int, struct qual);
 #ifdef INET6

diff --git a/grammar.y b/grammar.y
index 62f9b5f463ce2323b7b90aebd58ddb5bc9640d11..e3883b536e1b3e89472aaeec88edc839ba384a95 100644 (file)
--- a/grammar.y
+++ b/grammar.y
@@ -307,7 +307,6 @@ DIAG_OFF_BISON_BYACC
 %union {
        int i;
        bpf_u_int32 h;
-       u_char *e;
        char *s;
        struct stmt *stmt;
        struct arth *a;
@@ -363,9 +362,7 @@ DIAG_OFF_BISON_BYACC
 %token SIO OPC DPC SLS HSIO HOPC HDPC HSLS
 
 
-%type  <s> ID
-%type  <e> EID
-%type  <e> AID
+%type  <s> ID EID AID
 %type  <s> HID HID6
 %type  <i> NUM action reason type subtype type_subtype dir
 
@@ -437,24 +434,8 @@ nid:         ID                    { $$.b = gen_scode(cstate, $1, $$.q = $<blk>0.q); }
                                        "in this configuration");
 #endif /*INET6*/
                                }
-       | EID                   {
-                                 $$.b = gen_ecode(cstate, $1, $$.q = $<blk>0.q);
-                                 /*
-                                  * $1 was allocated by "pcap_ether_aton()",
-                                  * so we must free it now that we're done
-                                  * with it.
-                                  */
-                                 free($1);
-                               }
-       | AID                   {
-                                 $$.b = gen_acode(cstate, $1, $$.q = $<blk>0.q);
-                                 /*
-                                  * $1 was allocated by "pcap_ether_aton()",
-                                  * so we must free it now that we're done
-                                  * with it.
-                                  */
-                                 free($1);
-                               }
+       | EID                   { $$.b = gen_ecode(cstate, $1, $$.q = $<blk>0.q); }
+       | AID                   { $$.b = gen_acode(cstate, $1, $$.q = $<blk>0.q); }
        | not id                { gen_not($2.b); $$ = $2; }
        ;
 not:     '!'                   { $$ = $<blk>0; }

diff --git a/scanner.l b/scanner.l
index e0890b43b1b9d8f1777d275459e9166842d3db93..e9565aa63b3a160867fb302481a445a417eddff1 100644 (file)
--- a/scanner.l
+++ b/scanner.l
@@ -397,14 +397,8 @@ hsls               return HSLS;
 "=="                   return '=';
 "<<"                   return LSH;
 ">>"                   return RSH;
-${B}                   { yylval->e = pcap_ether_aton(((char *)yytext)+1);
-                         if (yylval->e == NULL)
-                               bpf_error(yyextra, "malloc");
-                         return AID; }
-{MAC}                  { yylval->e = pcap_ether_aton((char *)yytext);
-                         if (yylval->e == NULL)
-                               bpf_error(yyextra, "malloc");
-                         return EID; }
+${B}                   { yylval->s = sdup(yyextra, yytext); return AID; }
+{MAC}                  { yylval->s = sdup(yyextra, yytext); return EID; }
 {N}                    { yylval->i = stoi((char *)yytext); return NUM; }
 ({N}\.{N})|({N}\.{N}\.{N})|({N}\.{N}\.{N}\.{N})        {
                        yylval->s = sdup(yyextra, (char *)yytext); return HID; }