CVE-2017-12989/RESP: Make sure resp_get_length() advances the pointer for invalid...

Make sure that it always sends *endp before returning and that, for
invalid lengths where we don't like a character in the length string,
what it sets *endp to is past the character in question, so we don't
run the risk of infinitely looping (or doing something else random) if a
character in the length is invalid.

This fixes an infinite loop discovered by Forcepoint's security
researchers Otto Airamo & Antti Levomäki.

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

diff --git a/print-resp.c b/print-resp.c
index 9d71e21dcec5bb45341ce53525b7390f3c346b49..dee22d63a2fd1fbf323bac06a3dad7b0c097a99b 100644 (file)
--- a/print-resp.c
+++ b/print-resp.c
@@ -481,8 +481,10 @@ resp_get_length(netdissect_options *ndo, register const u_char *bp, int len, con
         ND_TCHECK(*bp);
         c = *bp;
         if (!(c >= '0' && c <= '9')) {
-            if (!saw_digit)
+            if (!saw_digit) {
+                bp++;
                 goto invalid;
+            }
             break;
         }
         c -= '0';
@@ -510,15 +512,19 @@ resp_get_length(netdissect_options *ndo, register const u_char *bp, int len, con
     if (len == 0)
         goto trunc;
     ND_TCHECK(*bp);
-    if (*bp != '\r')
+    if (*bp != '\r') {
+        bp++;
         goto invalid;
+    }
     bp++;
     len--;
     if (len == 0)
         goto trunc;
     ND_TCHECK(*bp);
-    if (*bp != '\n')
+    if (*bp != '\n') {
+        bp++;
         goto invalid;
+    }
     bp++;
     len--;
     *endp = bp;
@@ -531,8 +537,10 @@ resp_get_length(netdissect_options *ndo, register const u_char *bp, int len, con
     return (too_large ? -3 : result);
 
 trunc:
+    *endp = bp;
     return (-2);
 
 invalid:
+    *endp = bp;
     return (-5);
 }

diff --git a/tests/TESTLIST b/tests/TESTLIST
index e0f0bb1468ba6d72814e9c3d34391b842f2a4c49..ecf54f69bcc3ef41d28f342f3fcf2404dd8fb114 100644 (file)
--- a/tests/TESTLIST
+++ b/tests/TESTLIST
@@ -468,6 +468,7 @@ zephyr-oobr         zephyr-oobr.pcap                zephyr-oobr.out         -vvv -e
 bgp-as-path-oobr       bgp-as-path-oobr.pcap           bgp-as-path-oobr.out    -vvv -e
 isakmp-no-none-np      isakmp-no-none-np.pcap          isakmp-no-none-np.out   -vvv -e
 telnet-iac-check-oobr  telnet-iac-check-oobr.pcap      telnet-iac-check-oobr.out       -vvv -e
+resp_4_infiniteloop    resp_4_infiniteloop.pcap        resp_4_infiniteloop.out -vvv -e
 
 # RTP tests
 # fuzzed pcap

diff --git a/tests/resp_4_infiniteloop.out b/tests/resp_4_infiniteloop.out
new file mode 100644 (file)
index 0000000..396cb8b
--- /dev/null
+++ b/tests/resp_4_infiniteloop.out
@@ -0,0 +1,2 @@
+00:50:56:b4:08:69 > 00:50:56:b4:4c:2a, ethertype IPv4 (0x0800), length 920: (tos 0x0, ttl 64, id 27576, offset 0, flags [DF], proto TCP (6), length 906)
+    172.16.8.77.33926 > 172.16.8.149.6379: Flags [P.], cksum 0xa129 (incorrect -> 0xaaa0), seq 3839414413:3839415267, ack 2526552240, win 229, options [nop,nop,TS val 2407226 ecr 24894817], length 854: RESP length negative and not -1 invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid "4" "EVAL" invalid invalid invalid invalid "GKMbNZq^@0" "stuubt.pack('<ivdMFG4294967245',^V ''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''319', 2',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',',', '-1494241318543828858')'L')N))'r')')~D')')E)')')')')')')')'l')')')')')'M-`'o')')'Pp)U)" invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid invalid "1" [|RESP]

diff --git a/tests/resp_4_infiniteloop.pcap b/tests/resp_4_infiniteloop.pcap
new file mode 100644 (file)
index 0000000..98a79bf
Binary files /dev/null and b/tests/resp_4_infiniteloop.pcap differ