Added boundaries to the dns notation change functions

Possible boundaries violations found via Pareon Verify tool.

modules/pico_dns_client.c

@@ -414,8 +414,8 @@ static int pico_dns_client_user_callback(struct pico_dns_answer_suffix *asuffix,
     }
 #endif
     case PICO_DNS_TYPE_PTR:
-        pico_dns_notation_to_name(rdata);
-        str = PICO_ZALLOC((size_t)(asuffix->rdlength - PICO_DNS_LABEL_INITIAL));
+        pico_dns_notation_to_name(rdata, short_be(asuffix->rdlength));
+        str = PICO_ZALLOC((size_t)(short_be(asuffix->rdlength) - PICO_DNS_LABEL_INITIAL));
         if (!str) {
             pico_err = PICO_ERR_ENOMEM;
             return -1;
@@ -470,7 +470,7 @@ static void pico_dns_try_fallback_cname(struct pico_dns_query *q, struct pico_dn
     /* Found CNAME response. Re-initiating query. */
     asuffix = (struct pico_dns_answer_suffix *)p_asuffix;
     cname = (char *) asuffix + sizeof(struct pico_dns_answer_suffix);
-    pico_dns_notation_to_name(cname);
+    pico_dns_notation_to_name(cname, short_be(asuffix->rdlength));
     if (cname[0] == '.')
         cname++;
 
@@ -584,7 +584,7 @@ static int pico_dns_create_message(struct pico_dns_header **header, struct pico_
 
     /* assemble dns message */
     pico_dns_client_query_header(*header);
-    pico_dns_name_to_dns_notation(domain);
+    pico_dns_name_to_dns_notation(domain, strlen);
 
     return 0;
 }

modules/pico_dns_common.c

@@ -53,10 +53,11 @@ uint16_t pico_dns_client_strlen(const char *url)
 
 /* replace '.' in the domain name by the label length
  * f.e. www.google.be => 3www6google2be0 */
-int pico_dns_name_to_dns_notation(char *ptr)
+int pico_dns_name_to_dns_notation(char *ptr, unsigned int maxlen)
 {
     char p = 0, *label = NULL;
     uint8_t len = 0;
+    char *start = ptr;
 
     if (!ptr)
         return -1;
@@ -70,17 +71,19 @@ int pico_dns_name_to_dns_notation(char *ptr)
         } else {
             len++;
         }
+        if ((unsigned int)(ptr - start) > maxlen)
+            break;
     }
     *label = (char)len;
     return 0;
 }
 
 /* replace the label length in the domain name by '.'
  * f.e. 3www6google2be0 => .www.google.be */
-int pico_dns_notation_to_name(char *ptr)
+int pico_dns_notation_to_name(char *ptr, unsigned int maxlen)
 {
     char p = 0, *label = NULL;
-
+    char *start = ptr;
     if (!ptr)
         return -1;
 
@@ -89,6 +92,8 @@ int pico_dns_notation_to_name(char *ptr)
         ptr += p;
         *label = '.';
         label = ptr;
+        if ((unsigned int)(ptr - start) > maxlen)
+            break;
     }
     return 0;
 }

modules/pico_dns_common.h

@@ -90,8 +90,8 @@ enum pico_dns_arpa
 
 void pico_dns_fill_header(struct pico_dns_header *hdr, uint16_t qdcount, uint16_t ancount);
 uint16_t pico_dns_client_strlen(const char *url);
-int pico_dns_name_to_dns_notation(char *ptr);
-int pico_dns_notation_to_name(char *ptr);
+int pico_dns_name_to_dns_notation(char *ptr, unsigned int maxlen);
+int pico_dns_notation_to_name(char *ptr, unsigned int maxlen);
 void pico_dns_fill_query_suffix(struct pico_dns_query_suffix *suf, uint16_t type, uint16_t qclass);
 void pico_dns_fill_rr_suffix(struct pico_dns_answer_suffix *suf, uint16_t qtype, uint16_t qclass, uint32_t ttl, uint16_t rdlength);
 int8_t pico_dns_mirror_addr(char *ptr);

modules/pico_mdns.c

@@ -19,6 +19,7 @@
 #define PICO_MDNS_QUERY_TIMEOUT (10000) /* Ten seconds */
 #define PICO_MDNS_RR_TTL_TICK (1000) /* One second */
 #define PICO_MDNS_MAXBUF (1400)
+#define PICO_MDNS_CHARBUF_SIZE (256)
 
 #define mdns_dbg(...) do {} while(0)
 /*#define mdns_dbg dbg*/
@@ -158,7 +159,7 @@ static int pico_mdns_cache_del_rr(char *url, uint16_t qtype, char *rdata)
 static int pico_mdns_del_cookie(char *url, uint16_t qtype)
 {
     struct pico_mdns_cookie test, *found = NULL;
-    char temp[256] = {
+    char temp[PICO_MDNS_CHARBUF_SIZE] = {
         0
     };
     if(!url)
@@ -167,7 +168,7 @@ static int pico_mdns_del_cookie(char *url, uint16_t qtype)
     strcpy(temp + 1, url);
 
     test.url = temp;
-    pico_dns_name_to_dns_notation(test.url);
+    pico_dns_name_to_dns_notation(test.url, PICO_MDNS_CHARBUF_SIZE);
     test.qtype = qtype;
     found = pico_tree_findKey(&QTable, &test);
 
@@ -202,7 +203,7 @@ static void pico_mdns_cache_tick(pico_time now, void *_arg)
 static void pico_mdns_timeout(pico_time now, void *_arg)
 {
     struct pico_mdns_cookie *ck = (struct pico_mdns_cookie *)_arg;
-    char url[256] = {
+    char url[PICO_MDNS_CHARBUF_SIZE] = {
         0
     };
     IGNORE_PARAMETER(now);
@@ -212,7 +213,7 @@ static void pico_mdns_timeout(pico_time now, void *_arg)
 
     strcpy(url, ck->url);
 
-    pico_dns_notation_to_name(url);
+    pico_dns_notation_to_name(url, PICO_MDNS_CHARBUF_SIZE);
     pico_mdns_del_cookie(url + 1, ck->qtype);
 }
 
@@ -314,7 +315,7 @@ static struct pico_dns_header *pico_mdns_create_answer(char *url, unsigned int *
 
     /* assemble dns message */
     pico_mdns_fill_header(header, 0, 1); /* 0 questions, 1 answer */
-    pico_dns_name_to_dns_notation(domain);
+    pico_dns_name_to_dns_notation(domain, PICO_MDNS_CHARBUF_SIZE);
 
     pico_dns_fill_rr_suffix(asuffix, qtype, PICO_DNS_CLASS_IN, ttl, datalen);
 
@@ -435,7 +436,7 @@ static struct pico_dns_header *pico_mdns_create_query(const char *url, uint16_t
 
     /* assemble dns message */
     pico_mdns_fill_header(header, 1, 0);
-    pico_dns_name_to_dns_notation(domain);
+    pico_dns_name_to_dns_notation(domain, PICO_MDNS_CHARBUF_SIZE);
 
     if (pico_mdns_perform_query(qsuffix, proto, probe, inverse) < 0)
         return NULL;
@@ -449,7 +450,7 @@ static struct pico_mdns_cache_rr *pico_mdns_cache_find_rr(const char *url, uint1
     struct pico_mdns_cache_rr *rr = NULL;
     struct pico_dns_answer_suffix *suf = NULL;
     struct pico_mdns_cache_rr test;
-    char temp[256] = {
+    char temp[PICO_MDNS_CHARBUF_SIZE] = {
         0
     };
 
@@ -463,7 +464,7 @@ static struct pico_mdns_cache_rr *pico_mdns_cache_find_rr(const char *url, uint1
     strcpy(temp + 1, url);
     pico_to_lowercase(temp);
     test.url = temp;
-    pico_dns_name_to_dns_notation(test.url);
+    pico_dns_name_to_dns_notation(test.url, PICO_MDNS_CHARBUF_SIZE);
 
     mdns_dbg("Looking for '%s' with qtype '%d' in cache\n", url, qtype);
 
@@ -503,7 +504,7 @@ static int pico_mdns_cache_add_rr(char *url, struct pico_dns_answer_suffix *suf,
 
     memcpy(rr_url + 1, url, strlen(url));
     rr->url = rr_url;
-    pico_dns_name_to_dns_notation(rr->url);
+    pico_dns_name_to_dns_notation(rr->url, PICO_MDNS_CHARBUF_SIZE);
     memcpy(rr_suf, suf, sizeof(struct pico_dns_answer_suffix));
     rr->suf = rr_suf;
     rr->suf->qtype = short_be(rr->suf->qtype);
@@ -547,7 +548,7 @@ static int pico_mdns_cache_add_rr(char *url, struct pico_dns_answer_suffix *suf,
 static struct pico_mdns_cookie *pico_mdns_find_cookie(const char *url, uint16_t qtype)
 {
     struct pico_mdns_cookie test;
-    char temp[256] = {
+    char temp[PICO_MDNS_CHARBUF_SIZE] = {
         0
     };
 
@@ -557,7 +558,7 @@ static struct pico_mdns_cookie *pico_mdns_find_cookie(const char *url, uint16_t
     strcpy(temp + 1, url);
     pico_to_lowercase(temp);
     test.url = temp;
-    pico_dns_name_to_dns_notation(test.url);
+    pico_dns_name_to_dns_notation(test.url, PICO_MDNS_CHARBUF_SIZE);
     test.qtype = qtype;
     return pico_tree_findKey(&QTable, &test);
 }
@@ -598,12 +599,12 @@ static struct pico_dns_header *pico_mdns_query_create_answer(union pico_address
 #endif
     /* reply to PTR records */
     if(qtype == PICO_DNS_TYPE_PTR) {
-        char host_conv[255] = {
+        char host_conv[PICO_MDNS_CHARBUF_SIZE] = {
             0
         };
         mdns_dbg("Replying on PTR query...\n");
         strcpy(host_conv + 1, mdns_global_host);
-        pico_dns_name_to_dns_notation(host_conv);
+        pico_dns_name_to_dns_notation(host_conv, PICO_MDNS_CHARBUF_SIZE);
         return pico_mdns_create_answer(name, len, qtype, host_conv);
     }
 
@@ -727,7 +728,7 @@ static int pico_mdns_handle_answer(char *url, struct pico_dns_answer_suffix *suf
     }
 #endif
     else if(short_be(suf->qtype) == PICO_DNS_TYPE_PTR) {
-        pico_dns_notation_to_name(data);
+        pico_dns_notation_to_name(data, strlen(data)); /* This is not the right way, where do I get the lenght of the data field? */
         ck->callback(data + 1, ck->arg);    /* +1 to discard the beginning dot */
     }
     else {
@@ -845,7 +846,7 @@ static int pico_mdns_recv(void *buf, int buflen, struct pico_ip4 peer)
     /* handle queries */
     for(i = 0; i < qcount; i++) {
         qsuf = (struct pico_dns_query_suffix*) (ptr + pico_mdns_namelen_comp(ptr) + 1);
-        pico_dns_notation_to_name(ptr);
+        pico_dns_notation_to_name(ptr, PICO_MDNS_CHARBUF_SIZE);
         if (!ptr)
             return -1;
 

test/unit/modunit_pico_mdns.c

@@ -82,9 +82,9 @@ START_TEST(tc_pico_mdns_cache_del_rr)
     fail_unless(pico_mdns_cache_del_rr(url, qtype, rdata) == -1, "Deleted a nonexisting RR from cache!\n");
     fail_unless(pico_mdns_cache_add_rr(url, &suf, rdata) == 0, "Failed to add RR to cache\n");
 
-    addr = PICO_ZALLOC(strlen(url) + 1);
+    addr = PICO_ZALLOC(strlen(url) + 2);
     memcpy(addr + 1, url, strlen(url));
-    pico_dns_name_to_dns_notation(addr);
+    pico_dns_name_to_dns_notation(addr, strlen(url) + 1);
     fail_unless(pico_mdns_cache_del_rr(addr, qtype, rdata) == 0, "Unable to delete RR from cache!\n");
     PICO_FREE(addr);
 }
@@ -263,7 +263,7 @@ START_TEST(tc_pico_mdns_find_cookie)
     struct pico_dns_header *hdr = PICO_ZALLOC(sizeof(struct pico_dns_header) + strlen(url) + 1);
     addr = (char *)hdr + sizeof(struct pico_dns_header);
     memcpy(addr + 1, url, strlen(url));
-    pico_dns_name_to_dns_notation(addr);
+    pico_dns_name_to_dns_notation(addr, strlen(url));
 
     pico_stack_init();
     ck = pico_mdns_find_cookie(url, qtype);