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 --git a/print-radius.c b/print-radius.c
index 582795a12de4c194d0d6d882cbaa144e09ce2fe3..8555188ee9853a834aaa0cf57182f418b3e09002 100644 (file)
--- a/print-radius.c
+++ b/print-radius.c
@@ -503,10 +503,7 @@ print_attr_string(netdissect_options *ndo,
    {
       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
@@ -526,10 +523,7 @@ print_attr_string(netdissect_options *ndo,
            if (*data <= 0x1F)
            {
               if (length < 1)
-              {
-                 ND_PRINT((ndo, "%s", tstr));
-                 return;
-              }
+                 goto trunc;
               if (*data)
                 ND_PRINT((ndo, "Tag[%u] ", *data));
               else
@@ -539,6 +533,8 @@ print_attr_string(netdissect_options *ndo,
            }
         break;
       case EGRESS_VLAN_NAME:
+           if (length < 1)
+              goto trunc;
            ND_PRINT((ndo, "%s (0x%02x) ",
                   tok2str(rfc4675_tagged,"Unknown tag",*data),
                   *data));
@@ -547,7 +543,7 @@ print_attr_string(netdissect_options *ndo,
         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;

diff --git a/tests/TESTLIST b/tests/TESTLIST
index 9cb2bdb6bb167ab9a2596b5e0143c98d1104f569..314afa223804b75fa4b3505f5738bb66e622b08a 100644 (file)
--- a/tests/TESTLIST
+++ b/tests/TESTLIST
@@ -541,6 +541,7 @@ 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

diff --git a/tests/radius_attr_asan.out b/tests/radius_attr_asan.out
new file mode 100644 (file)
index 0000000..faef3dd
--- /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 (file)
index 0000000..9a7ed16
Binary files /dev/null and b/tests/radius_attr_asan.pcap differ