more range checks to avoid overrunning buffer for #6

mistakes-consortium · Aug 11, 2015 · bd63194 · bd63194
1 parent 3e8cd23
commit bd63194
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 2 deletions.
diff --git a/tests/testsuite.c b/tests/testsuite.c
@@ -12,6 +12,12 @@ unsigned char dns_response[] = {
     0xc0, 36, 0
 };
 
+// this one is too short.
+unsigned char dns_smallresponse[] = {
+    0, 0, 0, 0,
+    4, 't', 'e', 's', 't', 3, 'c', 'o', 'm', 0
+};
+
 void test_dnsname_no_compression()
 {
     // no dns compression, name compare simply and it works
@@ -27,10 +33,20 @@ void test_dnsname_no_compression()
     ASSERT_EQUALS(1, dns_compare_name("a.examplea.com.", dns_response, sizeof(dns_response), 12));
     // one segment too short....
     ASSERT_EQUALS(1, dns_compare_name("a.exampl.com.", dns_response, sizeof(dns_response), 12));
+}
+
+void test_malicious_input_rejected() {
     // no infinite loop....
     ASSERT_EQUALS(1, dns_compare_name(".", dns_response, sizeof(dns_response), 36));
 }
 
+void test_bounds_truncate() {
+    ASSERT_EQUALS(0, dns_compare_name("test.com", dns_smallresponse, sizeof(dns_smallresponse), 4));
+    ASSERT_EQUALS(1, dns_compare_name("test.com", dns_smallresponse, sizeof(dns_smallresponse) - 2, 4));
+    ASSERT_EQUALS(1, dns_compare_name("test.com", dns_smallresponse, sizeof(dns_smallresponse) - 1, 4));
+
+}
+
 void test_dnsname_with_compression()
 {
     // pointer to the part of the message that used DNS compression properly. False positive in stock LWIP...
@@ -48,5 +64,7 @@ int main()
 {
     RUN(test_dnsname_no_compression);
     RUN(test_dnsname_with_compression);
+    RUN(test_malicious_input_rejected);
+    RUN(test_bounds_truncate);
     return TEST_REPORT();
 }
diff --git a/user/dns_util.h b/user/dns_util.h
@@ -70,15 +70,23 @@ dns_compare_name(unsigned char *query, unsigned char *response_ptr, uint16_t res
             /** @see RFC 1035 - 4.1.4. Message compression */
             /* Compressed name */
             if (offset_bookmark != 0) {
-               //printf("<<<<<< this would be an infinite loop. no match, invalid formatting.\n");
-               return 1;
+                //printf("<<<<<< this would be an infinite loop. no match, invalid formatting.\n");
+                return 1;
             }
             offset_bookmark = offset + 1;
             offset = 255 * (0xc0 ^ n) + response[offset++];
             //printf("found offset to be %d after decoding pointer.\n", n);
+            if ((offset >= response_size) || (offset == 0)) {
+                //printf("<<<<<<<< pointer out of range (offset=%d). No match.\n", offset);
+                return 1;
+            }
         } else {
             /* Not compressed name */
             while (n > 0) {
+                if (offset >= response_size) {
+                    //printf("<<<<<< offset has grown larger than size of response. no match.\n");
+                    return 1;
+                }
                 if ((*query) != (response[offset])) {
                     //printf("<<<<<< a byte did not match. no match! Char in response was %c, in query was %c\n", response[offset], *query);
                     return 1;
@@ -95,6 +103,11 @@ dns_compare_name(unsigned char *query, unsigned char *response_ptr, uint16_t res
                 offset_bookmark = 0;
                 //printf("restored the offset bookmark to %d\n", offset);
             }
+
+            if (offset >= response_size) {
+                //printf("<<<<<< offset has grown larger than size of response. no match.\n");
+                return 1;
+            }
         }
 
     } while (response[offset] != 0);