CVE-2017-11541: In safeputs(), check the length before checking for a…

… NUL terminator. safeputs() doesn't do packet bounds checking of its own; it assumes that the caller has checked the availability in the packet data of all maxlen bytes of data. This means we should check that we're within the specified limit before looking at the byte. This fixes a buffer over-read discovered by Kamil Frankowicz. Add a test using the capture file supplied by the reporter(s).
the-tcpdump-group · Sep 2, 2017 · 09b1185 · 09b1185
1 parent 0d2332a
commit 09b1185
Show file tree

Hide file tree

Showing 4 changed files with 4 additions and 1 deletion.
diff --git a/tests/TESTLIST b/tests/TESTLIST
@@ -437,6 +437,7 @@ isoclns-heapoverflow-2	isoclns-heapoverflow-2.pcap	isoclns-heapoverflow-2.out	-e
 isoclns-heapoverflow-3	isoclns-heapoverflow-3.pcap	isoclns-heapoverflow-3.out	-e -c1
 stp-v4-length-sigsegv	stp-v4-length-sigsegv.pcap	stp-v4-length-sigsegv.out
 hoobr_pimv1		hoobr_pimv1.pcap		hoobr_pimv1.out
+hoobr_safeputs		hoobr_safeputs.pcap		hoobr_safeputs.out
 
 # RTP tests
 # fuzzed pcap

diff --git a/tests/hoobr_safeputs.out b/tests/hoobr_safeputs.out
@@ -0,0 +1,2 @@
+LLDP, length 808464418: 0000000000
+	[|LLDP]
diff --git a/tests/hoobr_safeputs.pcap b/tests/hoobr_safeputs.pcap
diff --git a/util-print.c b/util-print.c
@@ -902,7 +902,7 @@ safeputs(netdissect_options *ndo,
 {
 	u_int idx = 0;
 
-	while (*s && idx < maxlen) {
+	while (idx < maxlen && *s) {
 		safeputchar(ndo, *s);
 		idx++;
 		s++;