tcpdump crashes in print-cdp.c
Here is a new version of the diff that fixes the CDP printer in tcpdump.
(https://www.sccs.swarthmore.edu/users/16/mmcconv1/dump/tcpdump-crashes/)
With this change the code does not access outside the packet anymore
but it would be nice if it could be tested on a network with real CDP
(Cisco Discovery Protocol) packets to see if there is any change in the
output. Error checking may have become too aggressive.
Thanks,
Can
Index: print-cdp.c
===
RCS file: /cvs/src/usr.sbin/tcpdump/print-cdp.c,v
retrieving revision 1.5
diff -u -p -u -p -r1.5 print-cdp.c
--- print-cdp.c 16 Jan 2015 06:40:21 - 1.5
+++ print-cdp.c 23 Mar 2016 06:09:31 -
@@ -39,7 +39,7 @@
#include "addrtoname.h"
#include "extract.h" /* must come after interface.h */
-void cdp_print_addr(const u_char * p, int l);
+int cdp_print_addr(const u_char * p, int l);
void cdp_print_prefixes(const u_char * p, int l);
/*
@@ -65,7 +65,7 @@ cdp_print(const u_char *p, u_int length,
while (i < length) {
if (i + 4 > caplen) {
- printf("[!cdp]");
+ printf("[|cdp]");
return;
}
@@ -75,8 +75,11 @@ cdp_print(const u_char *p, u_int length,
if (vflag)
printf(" %02x/%02x", type, len);
+ if (len < 4)
+ goto error;
+
if (i+len > caplen) {
- printf("[!cdp]");
+ printf("[|cdp]");
return;
}
@@ -86,12 +89,15 @@ cdp_print(const u_char *p, u_int length,
break;
case 0x02:
printf(" Addr");
- cdp_print_addr(p + i + 4, len - 4);
+ if (cdp_print_addr(p + i + 4, len - 4))
+ goto error;
break;
case 0x03:
printf(" PortID '%.*s'", len - 4, p + i + 4);
break;
case 0x04:
+ if (len < 8)
+ goto error;
printf(" CAP 0x%02x", (unsigned) p[i+7]);
break;
case 0x05:
@@ -110,9 +116,13 @@ cdp_print(const u_char *p, u_int length,
printf(" VTP-Management-Domain '%.*s'", len-4, p+i+4 );
break;
case 0x0a: /* guess - not documented */
+ if (len < 6)
+ goto error;
printf(" Native-VLAN-ID %d", (p[i+4]<<8) + p[i+4+1] - 1
);
break;
case 0x0b: /* guess - not documented */
+ if (len < 5)
+ goto error;
printf(" Duplex %s", p[i+4] ? "full": "half" );
break;
default:
@@ -124,42 +134,59 @@ cdp_print(const u_char *p, u_int length,
break;
i += len;
}
+error:
+ printf("[!cdp]");
}
-void
+#define CDP_CHECK_ACCESS(p, s) if ((endp - (p)) < (s)) return 1
+
+int
cdp_print_addr(const u_char * p, int l)
{
- int pl, al, num;
+ int pl, pt, al, num;
const u_char * endp = p+l;
+ CDP_CHECK_ACCESS(p, 4);
num = (p[0] << 24) + (p[1]<<16) + (p[2]<<8)+ p[3];
p+=4;
printf(" (%d): ", num);
while(p < endp && num >= 0) {
+ CDP_CHECK_ACCESS(p, 2);
+ pt=*p;
pl=*(p+1);
p+=2;
+ CDP_CHECK_ACCESS(p, 3);
/* special case: IPv4, protocol type=0xcc, addr. length=4 */
if (pl == 1 && *p == 0xcc && p[1] == 0 && p[2] == 4) {
p+=3;
+ CDP_CHECK_ACCESS(p, 4);
printf("IPv4 %d.%d.%d.%d ", p[0], p[1], p[2], p[3]);
p+=4;
} else {/* generic case: just print raw data */
- printf("pt=0x%02x, pl=%d, pb=", *(p-2), pl);
+ printf("pt=0x%02x, pl=%d, pb=", pt, pl);
+
+ CDP_CHECK_ACCESS(p, pl);
while(pl-- > 0)
printf(" %02x", *p++);
+
+ CDP_CHECK_ACCESS(p, 2);
al=(*p << 8) + *(p+1);
printf(", al=%d, a=", al);
p+=2;
+
+ CDP_CHECK_ACCESS(p, al);
while(al-- > 0)
printf(" %02x", *p++);
}
printf(" ");
num--;
}
+
+ return 0;
}
@@ -169,7 +196,8 @@ cdp_print_prefixes(const u_char * p, int
printf(" IPv4 Prefixes
Test Request: variable-size DLT headers in libpcap
Please test the attached diff with tcpdump and pflogd
filter expressions you commonly use to make sure it
does not break anything.
This is needed for Henning's work on pflog.
Instructions:
1. Apply the patch under /usr/src/lib/libpcap,
2. re-build libpcap and install it.
3. restart pflogd
The patch changes the way BPF code is generated from
filter expressions for the pflog interface, to allow
for variable-sized pflog headers.
Things to test for:
1. Run your favorite filter expressions on both
pflog and non-pflog interfaces, and make sure
the packets are filtered correctly.
2. Make sure pflogd still generates correct logs,
especially if you are using a filter expression.
3. Try complex filter expressions on the pflog interface
and see if they still fit the memory (there is an
upper bound to the number of instructions, and this
diff increases the size of the generated code).
4. dump the result of some tcpdump filter code before
and after applying this patch for *non-pflog*
interfaces. There should be *no change*.
You can use '-d' switch to tcpdump to dump the filter code.
You can use '-O' to disable optimization.
Example: tcpdump -i em0 -d host 1.2.3.4 and port 80
Please let me know about your test results,
what you tested, and the architecture you tested on.
Background:
Years ago, when designing the new pflog header,
we assumed that the header would have to be
extended in the future. We did not want to switch
DLT numbers again, so we added a 'size' field
which also doubled as a 'version' field.
This would allow us to add fields to the pflog header
in the future and still be backwards compatible.
Unfortunately, our plans were foiled by the
mess that was libpcap. The bpf filtering language
and filter code generator in libpcap was designed
with the assumption that the link layer has a
fixed size (this is also the reason why there are
two 802_11 header types for WiFi).
Now that Henning has some real need to add to the
pflog header, he asked me to revisit that code and
try to get bpf filtering work on variable sized
pflog headers.
Technical details:
BPF uses a very simple language for testing packets
against a filter language. The filter expression
you specify in tcpdump is parsed and assembled into
a bytecode that the kernel can use to match packets.
Each interface has a specific link-type (DLT_EN10MB
for ethernet, DLT_PFLOG for pflog etc.) and each
link type has a header infront of the packet itself.
The code generation itself is done in libpcap(4)
for simplicity, the size of the link-header for
each link-type is hardcoded into the code generator.
The diff modifies the code generator to handle
variable-sized link-headers. For pflog, which has
the header length in the header itself, the generator
generates additional BPF code to parse the pflog
link-header to determine the size of the link header
instead of assuming a fixed size.
The two important offsets based on the link header
are 1) the start of the link payload (eg. IP header)
and 2) the start of the protocol header (eg. TCP header)
these values require a number of BPF instructions to
compute, and used extensively by most filter constructs.
To prevent the code size from increasing too much, the
offsets are computed once, at the start, and the results
are placed in 'registers'.
Let me know if you have any questions.
Thanks,
Can
Index: gencode.c
===
RCS file: /cvs/src/lib/libpcap/gencode.c,v
retrieving revision 1.33
diff -u -p -r1.33 gencode.c
--- gencode.c 26 Jun 2010 16:47:07 - 1.33
+++ gencode.c 1 Jul 2010 04:51:46 -
@@ -99,6 +99,13 @@ static void free_reg(int);
static struct block *root;
+/* initialization code used for variable link header */
+static struct slist *init_code = NULL;
+
+/* Flags and registers for variable link type handling */
+static int variable_nl;
+static int nl_reg, iphl_reg;
+
/*
* We divy out chunks of memory rather than call malloc each time so
* we don't have to worry about leaking memory. It's probably
@@ -126,7 +133,9 @@ static void backpatch(struct block *, st
static void merge(struct block *, struct block *);
static struct block *gen_cmp(u_int, u_int, bpf_int32);
static struct block *gen_cmp_gt(u_int, u_int, bpf_int32);
+static struct block *gen_cmp_nl(u_int, u_int, bpf_int32);
static struct block *gen_mcmp(u_int, u_int, bpf_int32, bpf_u_int32);
+static struct block *gen_mcmp_nl(u_int, u_int, bpf_int32, bpf_u_int32);
static struct block *gen_bcmp(u_int, u_int, const u_char *);
static struct block *gen_uncond(int);
static __inline struct block *gen_true(void);
@@ -296,6 +305,12 @@ pcap_compile(pcap_t *p, struct bpf_progr
if (root == NULL)
root = gen_retblk(snaplen);
+ /* prepend initialization code to root */
+ if (init_code != NULL root != NULL) {
+ sappend(init_code, root-stmts);
+ root-stmts =
