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 --git a/print-pgm.c b/print-pgm.c
index c22b188b7c02d9d0d34383cd9f43c71a53add52c..8340f2c3eb43f5d05f0ba7048f78efbfa84e6635 100644 (file)
--- a/print-pgm.c
+++ b/print-pgm.c
@@ -169,13 +169,12 @@ pgm_print(netdissect_options *ndo,
                        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);
@@ -362,6 +361,7 @@ pgm_print(netdissect_options *ndo,
             * 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));

diff --git a/tests/TESTLIST b/tests/TESTLIST
index 433a8fab94273c28004c7cadfbf32736cda5eede..1c94dcca1669ccb43d18db75039d44f957e4678f 100644 (file)
--- a/tests/TESTLIST
+++ b/tests/TESTLIST
@@ -522,6 +522,7 @@ 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

diff --git a/tests/pgm_group_addr_asan.out b/tests/pgm_group_addr_asan.out
new file mode 100644 (file)
index 0000000..a09d9b2
--- /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 (file)
index 0000000..5c11567
Binary files /dev/null and b/tests/pgm_group_addr_asan.pcap differ