CVE-2017-13032: Out-of-bounds Read (CWE-125)
value: The RADIUS parser in tcpdump before 4.9.2 has a buffer over-read in print-radius.c:print_attr_string().
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-radius.c, tests/TESTLIST, tests/radius_attr_asan.out
Commit date: 2017-09-13 12:25:44 +0100
Commit message: CVE-2017-13032/RADIUS: Check whether a byte exists before testing its value.

Reverse the test in a for loop to test the length before testing whether
we have a null byte.

This fixes a buffer over-read discovered by Bhargava Shastry.

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.

Clean up other length tests while we're at it.

Diff: diff --git a/print-radius.c b/print-radius.c
index 582795a12d..8555188ee9 100644
--- a/print-radius.c
+++ b/print-radius.c
@@ -501,14 +501,11 @@ print_attr_string(netdissect_options *ndo,
 
    switch(attr_code)
    {
       case TUNNEL_PASS:
            if (length < 3)
-           {
-              ND_PRINT((ndo, "%s", tstr));
-              return;
-           }
+              goto trunc;
            if (*data && (*data <=0x1F) )
               ND_PRINT((ndo, "Tag[%u] ", *data));
            else
               ND_PRINT((ndo, "Tag[Unused] "));
            data++;
@@ -524,32 +521,31 @@ print_attr_string(netdissect_options *ndo,
       case TUNNEL_CLIENT_AUTH:
       case TUNNEL_SERVER_AUTH:
            if (*data <= 0x1F)
            {
               if (length < 1)
-              {
-                 ND_PRINT((ndo, "%s", tstr));
-                 return;
-              }
+                 goto trunc;
               if (*data)
                 ND_PRINT((ndo, "Tag[%u] ", *data));
               else
                 ND_PRINT((ndo, "Tag[Unused] "));
               data++;
               length--;
            }
         break;
       case EGRESS_VLAN_NAME:
+           if (length < 1)
+              goto trunc;
            ND_PRINT((ndo, "%s (0x%02x) ",
                   tok2str(rfc4675_tagged,"Unknown tag",*data),
                   *data));
            data++;
            length--;
         break;
    }
 
-   for (i=0; *data && i < length ; i++, data++)
+   for (i=0; i < length && *data; i++, data++)
        ND_PRINT((ndo, "%c", (*data < 32 || *data > 126) ? '.' : *data));
 
    return;
 
    trunc:
diff --git a/tests/TESTLIST b/tests/TESTLIST
index 9cb2bdb6bb..314afa2238 100644
--- a/tests/TESTLIST
+++ b/tests/TESTLIST
@@ -539,10 +539,11 @@ bootp_asan		bootp_asan.pcap			bootp_asan.out		-v
 ppp_ccp_config_deflate_option_asan	ppp_ccp_config_deflate_option_asan.pcap	ppp_ccp_config_deflate_option_asan.out	-v
 pim_header_asan		pim_header_asan.pcap		pim_header_asan.out	-v
 pim_header_asan-2	pim_header_asan-2.pcap		pim_header_asan-2.out	-v
 pim_header_asan-3	pim_header_asan-3.pcap		pim_header_asan-3.out	-v
 ip6_frag_asan		ip6_frag_asan.pcap		ip6_frag_asan.out	-v
+radius_attr_asan	radius_attr_asan.pcap		radius_attr_asan.out	-v
 
 # RTP tests
 # fuzzed pcap
 rtp-seg-fault-1  rtp-seg-fault-1.pcap  rtp-seg-fault-1.out  -v -T rtp
 rtp-seg-fault-2  rtp-seg-fault-2.pcap  rtp-seg-fault-2.out  -v -T rtp
diff --git a/tests/radius_attr_asan.out b/tests/radius_attr_asan.out
new file mode 100644
index 0000000000..faef3ddf16
--- /dev/null
+++ b/tests/radius_attr_asan.out
@@ -0,0 +1,9 @@
+IP (tos 0x64, ttl 249, id 40192, offset 0, flags [+, DF, rsvd], proto UDP (17), length 299, options (unknown 235 [bad length 252]), bad cksum 8000 (->1faa)!)
+    0.0.86.32.258 > 0.2.250.99.3799: RADIUS, length: 263
+	Unknown Command (58), id: 0x6a, Authenticator: 0901020ed7ff03edb63a0f00cb0f00cb
+	  NAS-Port Attribute (5), length: 5, Value: ERROR: length 3 != 4
+	  Unknown Attribute (127), length: 4, Value: 
+	  NAS-IP-Address Attribute (4), length: 4, Value: ERROR: length 2 != 4
+	  NAS-IP-Address Attribute (4), length: 4, Value: ERROR: length 2 != 4
+	  NAS-IP-Address Attribute (4), length: 4, Value: ERROR: length 2 != 4
+	  Callback-Id Attribute (20), length: 4, Value: .. [|radius]
diff --git a/tests/radius_attr_asan.pcap b/tests/radius_attr_asan.pcap
new file mode 100644
index 0000000000..9a7ed160d0
Binary files /dev/null and b/tests/radius_attr_asan.pcap differ

Vulnerable code:            {
              ND_PRINT((ndo, "%s", tstr));
              return;
           }
              {
                 ND_PRINT((ndo, "%s", tstr));
                 return;
              }
   for (i=0; *data && i < length ; i++, data++)
Fixed code:               goto trunc;
                 goto trunc;
           if (length < 1)
              goto trunc;
   for (i=0; i < length && *data; i++, data++)
radius_attr_asan	radius_attr_asan.pcap		radius_attr_asan.out	-v
IP (tos 0x64, ttl 249, id 40192, offset 0, flags [+, DF, rsvd], proto UDP (17), length 299, options (unknown 235 [bad length 252]), bad cksum 8000 (->1faa)!)
    0.0.86.32.258 > 0.2.250.99.3799: RADIUS, length: 263
	Unknown Command (58), id: 0x6a, Authenticator: 0901020ed7ff03edb63a0f00cb0f00cb
	  NAS-Port Attribute (5), length: 5, Value: ERROR: length 3 != 4
	  Unknown Attribute (127), length: 4, Value: 
	  NAS-IP-Address Attribute (4), length: 4, Value: ERROR: length 2 != 4
	  NAS-IP-Address Attribute (4), length: 4, Value: ERROR: length 2 != 4
	  NAS-IP-Address Attribute (4), length: 4, Value: ERROR: length 2 != 4
	  Callback-Id Attribute (20), length: 4, Value: .. [|radius]