CVE-2017-13010: Out-of-bounds Read (CWE-125)
value: The BEEP parser in tcpdump before 4.9.2 has a buffer over-read in print-beep.c:l_strnstart().
CWE description: The product reads data past the end, or before the beginning, of the intended buffer.
CVSS v3 base score: 9.8
Published: 2017-09-14T06:29Z
Repository: https://github.com/the-tcpdump-group/tcpdump
Language: Other
Files changed: print-beep.c, tests/TESTLIST, tests/beep-oobr.out
Commit date: 2017-09-13 12:25:44 +0100
Commit message: CVE-2017-13010/BEEP: Do bounds checking when comparing strings.

This fixes a buffer over-read discovered by Brian 'geeknik' Carpenter.

Add a test using the capture file supplied by the reporter(s).

Diff: diff --git a/print-beep.c b/print-beep.c
index ed502b96c..64a162d74 100644
--- a/print-beep.c
+++ b/print-beep.c
@@ -26,35 +26,43 @@
  *
  * Looks at the first few chars up to tl1 ...
  */
 
 static int
-l_strnstart(const char *tstr1, u_int tl1, const char *str2, u_int l2)
+l_strnstart(netdissect_options *ndo, const char *tstr1, u_int tl1,
+    const char *str2, u_int l2)
 {
-
+	if (!ND_TTEST2(*str2, tl1)) {
+		/*
+		 * We don't have tl1 bytes worth of captured data
+		 * for the string, so we can't check for this
+		 * string.
+		 */
+		return 0;
+	}
 	if (tl1 > l2)
 		return 0;
 
 	return (strncmp(tstr1, str2, tl1) == 0 ? 1 : 0);
 }
 
 void
 beep_print(netdissect_options *ndo, const u_char *bp, u_int length)
 {
 
-	if (l_strnstart("MSG", 4, (const char *)bp, length)) /* A REQuest */
+	if (l_strnstart(ndo, "MSG", 4, (const char *)bp, length)) /* A REQuest */
 		ND_PRINT((ndo, " BEEP MSG"));
-	else if (l_strnstart("RPY ", 4, (const char *)bp, length))
+	else if (l_strnstart(ndo, "RPY ", 4, (const char *)bp, length))
 		ND_PRINT((ndo, " BEEP RPY"));
-	else if (l_strnstart("ERR ", 4, (const char *)bp, length))
+	else if (l_strnstart(ndo, "ERR ", 4, (const char *)bp, length))
 		ND_PRINT((ndo, " BEEP ERR"));
-	else if (l_strnstart("ANS ", 4, (const char *)bp, length))
+	else if (l_strnstart(ndo, "ANS ", 4, (const char *)bp, length))
 		ND_PRINT((ndo, " BEEP ANS"));
-	else if (l_strnstart("NUL ", 4, (const char *)bp, length))
+	else if (l_strnstart(ndo, "NUL ", 4, (const char *)bp, length))
 		ND_PRINT((ndo, " BEEP NUL"));
-	else if (l_strnstart("SEQ ", 4, (const char *)bp, length))
+	else if (l_strnstart(ndo, "SEQ ", 4, (const char *)bp, length))
 		ND_PRINT((ndo, " BEEP SEQ"));
-	else if (l_strnstart("END", 4, (const char *)bp, length))
+	else if (l_strnstart(ndo, "END", 4, (const char *)bp, length))
 		ND_PRINT((ndo, " BEEP END"));
 	else
 		ND_PRINT((ndo, " BEEP (payload or undecoded)"));
 }
diff --git a/tests/TESTLIST b/tests/TESTLIST
index 5dad008ef..080a00fbd 100644
--- a/tests/TESTLIST
+++ b/tests/TESTLIST
@@ -438,10 +438,11 @@ ipv6-rthdr-oobr		ipv6-rthdr-oobr.pcap		ipv6-rthdr-oobr.out
 ieee802.11_tim_ie_oobr	ieee802.11_tim_ie_oobr.pcap	ieee802.11_tim_ie_oobr.out
 decnet-shorthdr-oobr	decnet-shorthdr-oobr.pcap	decnet-shorthdr-oobr.out
 isakmp-3948-oobr-2	isakmp-3948-oobr-2.pcap		isakmp-3948-oobr-2.out
 ieee802.11_rates_oobr	ieee802.11_rates_oobr.pcap	ieee802.11_rates_oobr.out
 ipv6-mobility-header-oobr	ipv6-mobility-header-oobr.pcap	ipv6-mobility-header-oobr.out
+beep-oobr		beep-oobr.pcap			beep-oobr.out
 
 # bad packets from Kamil Frankowicz
 snmp-heapoverflow-1	snmp-heapoverflow-1.pcap	snmp-heapoverflow-1.out
 snmp-heapoverflow-2	snmp-heapoverflow-2.pcap	snmp-heapoverflow-2.out
 isoclns-heapoverflow-2	isoclns-heapoverflow-2.pcap	isoclns-heapoverflow-2.out	-e -c1
diff --git a/tests/beep-oobr.out b/tests/beep-oobr.out
new file mode 100644
index 000000000..ba46f2899
--- /dev/null
+++ b/tests/beep-oobr.out
@@ -0,0 +1,2 @@
+unknown ip 3
+IP6 3030:3030:3030:3030:3030:3030:3030:3030.10288 > 3030:3030:3030:3030:3030:3030:3030:3030.12336: Flags [.U], seq 808464432:808476740, ack 808464432, win 12336, urg 12336, options [eol], length 12308 BEEP (payload or undecoded)
diff --git a/tests/beep-oobr.pcap b/tests/beep-oobr.pcap
new file mode 100644
index 000000000..ea3853b45
Binary files /dev/null and b/tests/beep-oobr.pcap differ

Vulnerable code: l_strnstart(const char *tstr1, u_int tl1, const char *str2, u_int l2)

	if (l_strnstart("MSG", 4, (const char *)bp, length)) /* A REQuest */
	else if (l_strnstart("RPY ", 4, (const char *)bp, length))
	else if (l_strnstart("ERR ", 4, (const char *)bp, length))
	else if (l_strnstart("ANS ", 4, (const char *)bp, length))
	else if (l_strnstart("NUL ", 4, (const char *)bp, length))
	else if (l_strnstart("SEQ ", 4, (const char *)bp, length))
	else if (l_strnstart("END", 4, (const char *)bp, length))
Fixed code: l_strnstart(netdissect_options *ndo, const char *tstr1, u_int tl1,
    const char *str2, u_int l2)
	if (!ND_TTEST2(*str2, tl1)) {
		/*
		 * We don't have tl1 bytes worth of captured data
		 * for the string, so we can't check for this
		 * string.
		 */
		return 0;
	}
	if (l_strnstart(ndo, "MSG", 4, (const char *)bp, length)) /* A REQuest */
	else if (l_strnstart(ndo, "RPY ", 4, (const char *)bp, length))
	else if (l_strnstart(ndo, "ERR ", 4, (const char *)bp, length))
	else if (l_strnstart(ndo, "ANS ", 4, (const char *)bp, length))
	else if (l_strnstart(ndo, "NUL ", 4, (const char *)bp, length))
	else if (l_strnstart(ndo, "SEQ ", 4, (const char *)bp, length))
	else if (l_strnstart(ndo, "END", 4, (const char *)bp, length))
beep-oobr		beep-oobr.pcap			beep-oobr.out
unknown ip 3
IP6 3030:3030:3030:3030:3030:3030:3030:3030.10288 > 3030:3030:3030:3030:3030:3030:3030:3030.12336: Flags [.U], seq 808464432:808476740, ack 808464432, win 12336, urg 12336, options [eol], length 12308 BEEP (payload or undecoded)