CVE-2017-13034: Out-of-bounds Read (CWE-125)
value: The PGM parser in tcpdump before 4.9.2 has a buffer over-read in print-pgm.c:pgm_print().
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-pgm.c, tests/TESTLIST, tests/pgm_group_addr_asan.out
Commit date: 2017-09-13 12:25:44 +0100
Commit message: CVE-2017-13034/PGM: Add a bounds check.

This fixes a buffer over-read discovered by Bhargava Shastry,
SecT/TU Berlin.

Add a test using the capture file supplied by the reporter(s), modified
so the capture file won't be rejected as an invalid capture.

Move a return to make the code a bit cleaner (i.e., make it more obvious
that if we don't have enough of the PGM header, we just print the source
and destination IP addresses, mark it as incomplete PGM, and don't try
to look at the PGM header).

Diff: diff --git a/print-pgm.c b/print-pgm.c
index c22b188b7c..8340f2c3eb 100644
--- a/print-pgm.c
+++ b/print-pgm.c
@@ -167,17 +167,16 @@ pgm_print(netdissect_options *ndo,
 	if (!ND_TTEST(pgm->pgm_dport)) {
 		if (ip6) {
 			ND_PRINT((ndo, "%s > %s: [|pgm]",
 				ip6addr_string(ndo, &ip6->ip6_src),
 				ip6addr_string(ndo, &ip6->ip6_dst)));
-			return;
 		} else {
 			ND_PRINT((ndo, "%s > %s: [|pgm]",
 				ipaddr_string(ndo, &ip->ip_src),
 				ipaddr_string(ndo, &ip->ip_dst)));
-			return;
 		}
+		return;
 	}
 
 	sport = EXTRACT_16BITS(&pgm->pgm_sport);
 	dport = EXTRACT_16BITS(&pgm->pgm_dport);
 
@@ -360,10 +359,11 @@ pgm_print(netdissect_options *ndo,
 	    /*
 	     * Skip past the group, saving info along the way
 	     * and stopping if we don't have enough.
 	     */
 	    bp += (2 * sizeof(uint16_t));
+	    ND_TCHECK_16BITS(bp);
 	    switch (EXTRACT_16BITS(bp)) {
 	    case AFNUM_INET:
 		ND_TCHECK2(*bp, sizeof(struct in_addr));
 		addrtostr(bp, group_buf, sizeof(group_buf));
 		bp += sizeof(struct in_addr);
diff --git a/tests/TESTLIST b/tests/TESTLIST
index 433a8fab94..1c94dcca16 100644
--- a/tests/TESTLIST
+++ b/tests/TESTLIST
@@ -520,10 +520,11 @@ esis_snpa_asan-4	esis_snpa_asan-4.pcap		esis_snpa_asan-4.out	-v
 esis_snpa_asan-5	esis_snpa_asan-5.pcap		esis_snpa_asan-5.out	-v
 dhcp6_reconf_asan	dhcp6_reconf_asan.pcap		dhcp6_reconf_asan.out	-v
 pgm_opts_asan		pgm_opts_asan.pcap		pgm_opts_asan.out	-v
 pgm_opts_asan_2		pgm_opts_asan_2.pcap		pgm_opts_asan_2.out	-v
 pgm_opts_asan_3		pgm_opts_asan_3.pcap		pgm_opts_asan_3.out	-v
+pgm_group_addr_asan	pgm_group_addr_asan.pcap	pgm_group_addr_asan.out	-v
 vtp_asan		vtp_asan.pcap			vtp_asan.out	-v
 vtp_asan-2		vtp_asan-2.pcap			vtp_asan-2.out	-v
 vtp_asan-3		vtp_asan-3.pcap			vtp_asan-3.out	-v
 icmp6_mobileprefix_asan	icmp6_mobileprefix_asan.pcap	icmp6_mobileprefix_asan.out	-v
 ip_printroute_asan	ip_printroute_asan.pcap		ip_printroute_asan.out	-v
diff --git a/tests/pgm_group_addr_asan.out b/tests/pgm_group_addr_asan.out
new file mode 100644
index 0000000000..a09d9b2ac4
--- /dev/null
+++ b/tests/pgm_group_addr_asan.out
@@ -0,0 +1,2 @@
+IP (tos 0x41,ECT(1), id 40256, offset 0, flags [none], proto PGM (113), length 768, options (unknown 89 [bad length 232]), bad cksum 5959 (->5afd)!)
+    89.121.89.107 > 89.89.89.89: 89.121.89.107.32322 > 89.89.89.89.500: PGM, length 24818 0x00ff7f010347 [|pgm]
diff --git a/tests/pgm_group_addr_asan.pcap b/tests/pgm_group_addr_asan.pcap
new file mode 100644
index 0000000000..5c11567198
Binary files /dev/null and b/tests/pgm_group_addr_asan.pcap differ

Vulnerable code: 			return;
			return;
Fixed code: 		return;
	    ND_TCHECK_16BITS(bp);
pgm_group_addr_asan	pgm_group_addr_asan.pcap	pgm_group_addr_asan.out	-v
IP (tos 0x41,ECT(1), id 40256, offset 0, flags [none], proto PGM (113), length 768, options (unknown 89 [bad length 232]), bad cksum 5959 (->5afd)!)
    89.121.89.107 > 89.89.89.89: 89.121.89.107.32322 > 89.89.89.89.500: PGM, length 24818 0x00ff7f010347 [|pgm]