CVE-2017-5484/ATM: fix an incorrect bounds check

The function sig_print() did receive a correct caplen parameter value
but didn't use it correctly and could overread by one byte as Brian
Carpenter has demonstrated. Fix it by switching to the standard macros.

diff --git a/print-atm.c b/print-atm.c
index 058541c2128ed1dd1195be57d640e6f38d748933..596e406ccab63261f16178babcb5a8a8043e2e76 100644 (file)
--- a/print-atm.c
+++ b/print-atm.c
@@ -345,24 +345,18 @@ static const struct tok msgtype2str[] = {
 
 static void
 sig_print(netdissect_options *ndo,
-          const u_char *p, int caplen)
+          const u_char *p)
 {
        uint32_t call_ref;
 
-       if (caplen < PROTO_POS) {
-               ND_PRINT((ndo, "%s", tstr));
-               return;
-       }
+       ND_TCHECK(p[PROTO_POS]);
        if (p[PROTO_POS] == Q2931) {
                /*
                 * protocol:Q.2931 for User to Network Interface
                 * (UNI 3.1) signalling
                 */
                ND_PRINT((ndo, "Q.2931"));
-               if (caplen < MSG_TYPE_POS) {
-                       ND_PRINT((ndo, " %s", tstr));
-                       return;
-               }
+               ND_TCHECK(p[MSG_TYPE_POS]);
                ND_PRINT((ndo, ":%s ",
                    tok2str(msgtype2str, "msgtype#%d", p[MSG_TYPE_POS])));
 
@@ -378,6 +372,10 @@ sig_print(netdissect_options *ndo,
                /* SCCOP with some unknown protocol atop it */
                ND_PRINT((ndo, "SSCOP, proto %d ", p[PROTO_POS]));
        }
+       return;
+
+trunc:
+       ND_PRINT((ndo, " %s", tstr));
 }
 
 /*
@@ -395,7 +393,7 @@ atm_print(netdissect_options *ndo,
                switch (vci) {
 
                case VCI_PPC:
-                       sig_print(ndo, p, caplen);
+                       sig_print(ndo, p);
                        return;
 
                case VCI_BCC:

diff --git a/tests/TESTLIST b/tests/TESTLIST
index e8856c01717313adab8a7dedeccef9874f735ab0..8808a3fa352901f94f22b4f2b743b409a82182f0 100644 (file)
--- a/tests/TESTLIST
+++ b/tests/TESTLIST
@@ -425,6 +425,7 @@ ipv6hdr-heapoverflow-v      ipv6hdr-heapoverflow.pcap       ipv6hdr-heapoverflow-v.out      -t -
 otv-heapoverflow-1     otv-heapoverflow-1.pcap         otv-heapoverflow-1.out          -t -c10
 otv-heapoverflow-2     otv-heapoverflow-2.pcap         otv-heapoverflow-2.out          -t -c10
 q933-heapoverflow-2    q933-heapoverflow-2.pcap        q933-heapoverflow-2.out         -t
+atm-heapoverflow       atm-heapoverflow.pcap           atm-heapoverflow.out            -t -c1 -e
 
 # bad packets from Kamil Frankowicz
 snmp-heapoverflow-1    snmp-heapoverflow-1.pcap        snmp-heapoverflow-1.out         -t

diff --git a/tests/atm-heapoverflow.out b/tests/atm-heapoverflow.out
new file mode 100644 (file)
index 0000000..c9b12fd
--- /dev/null
+++ b/tests/atm-heapoverflow.out
@@ -0,0 +1 @@
+Rx: VPI:0 VCI:5  [|atm]

diff --git a/tests/atm-heapoverflow.pcap b/tests/atm-heapoverflow.pcap
new file mode 100644 (file)
index 0000000..6918f3e
Binary files /dev/null and b/tests/atm-heapoverflow.pcap differ