From: Guy Harris Date: Wed, 17 Oct 2018 07:59:40 +0000 (-0700) Subject: Plug some memory leaks. X-Git-Tag: libpcap-1.10-bp~769 X-Git-Url: https://git.tcpdump.org/libpcap/commitdiff_plain/bcbef226ca11662342b5e267e7f12066bcfd60d0 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 1b20124b..7aa96387 100644 --- 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 6a6fda58..e97e90fe 100644 --- 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 62f9b5f4..e3883b53 100644 --- 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 ID -%type EID -%type AID +%type ID EID AID %type HID HID6 %type NUM action reason type subtype type_subtype dir @@ -437,24 +434,8 @@ nid: ID { $$.b = gen_scode(cstate, $1, $$.q = $0.q); } "in this configuration"); #endif /*INET6*/ } - | EID { - $$.b = gen_ecode(cstate, $1, $$.q = $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 = $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 = $0.q); } + | AID { $$.b = gen_acode(cstate, $1, $$.q = $0.q); } | not id { gen_not($2.b); $$ = $2; } ; not: '!' { $$ = $0; } diff --git a/scanner.l b/scanner.l index e0890b43..e9565aa6 100644 --- 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; }