packages/libpcap
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity
libpcap/0194-Plug-some-memory-leaks.patch
overweight 81b8d9f850 Package init
2019-09-30 10:57:32 -04:00

235 lines
7.2 KiB
Diff
Raw Blame History

From bcbef226ca11662342b5e267e7f12066bcfd60d0 Mon Sep 17 00:00:00 2001
From: Guy Harris <guy@alum.mit.edu>
Date: Wed, 17 Oct 2018 00:59:40 -0700
Subject: [PATCH 194/470] 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.
---
gencode.c | 53 ++++++++++++++++++++++++++++++++++++++++++-----------
gencode.h | 4 ++--
grammar.y | 25 +++----------------------
scanner.l | 10 ++--------
4 files changed, 49 insertions(+), 43 deletions(-)
diff --git a/gencode.c b/gencode.c
index 1b20124..7aa9638 100644
--- a/gencode.c
+++ b/gencode.c
@@ -279,6 +279,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
* of the packet data.
@@ -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 6a6fda5..e97e90f 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 62f9b5f..e3883b5 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 <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 e0890b4..e9565aa 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; }
--
1.8.3.1
Reference in New Issue View Git Blame Copy Permalink