Add bounds checking to the functions building certs packets

Author: jedisct1

dnscrypt.c

@@ -391,7 +391,7 @@ dnscrypt_server_curve(struct context *c, const dnsccert *cert,
  */
 int
 dnscrypt_self_serve_cert_file(struct context *c, struct dns_header *header,
-                              size_t *dns_query_len)
+                              size_t *dns_query_len, size_t max_len)
 {
     unsigned char *p;
     unsigned char *ansp;
@@ -438,7 +438,7 @@ dnscrypt_self_serve_cert_file(struct context *c, struct dns_header *header,
 
         for (int i=0; i < c->signed_certs_count; i++) {
             if (add_resource_record
-                (header, nameoffset, &ansp, 0, NULL, T_TXT, C_IN, "t", size,
+                (header, nameoffset, max_len, &ansp, 0, NULL, T_TXT, C_IN, "t", size,
                     *(txt + i))) {
                 anscount++;
             } else {

dnscrypt.h

@@ -237,6 +237,6 @@ int dnscrypt_server_curve(struct context *c, const dnsccert *cert,
  * */
 int dnscrypt_self_serve_cert_file(struct context *c,
                                   struct dns_header *header,
-                                  size_t *dns_query_len);
+                                  size_t *dns_query_len, size_t max_len);
 
 #endif

rfc1035.c

@@ -186,7 +186,7 @@ questions_hash(uint64_t *hash, struct dns_header *header, size_t plen,
         (name_len = strlen(name)) > MAXDNAME) {
         return -1;
     }
-    crypto_shorthash((unsigned char *) hash, name, name_len, key);
+    crypto_shorthash((unsigned char *) hash, (const unsigned char *) name, name_len, key);
 
     return 0;
 }
@@ -278,7 +278,7 @@ do_rfc1035_name(unsigned char *p, char *sval)
 }
 
 int
-add_resource_record(struct dns_header *header, unsigned int nameoffset,
+add_resource_record(struct dns_header *header, unsigned int nameoffset, size_t plen,
                     unsigned char **pp, unsigned long ttl, unsigned int *offset,
                     unsigned short type, unsigned short class, char *format,
                     ...)
@@ -290,6 +290,9 @@ add_resource_record(struct dns_header *header, unsigned int nameoffset,
     long           lval;
     char          *sval;
 
+    if (!CHECK_LEN(header, p, plen, 12)) {
+        return 0;
+    }
     PUTSHORT(nameoffset | 0xc000, p);
     PUTSHORT(type, p);
     PUTSHORT(class, p);
@@ -304,24 +307,36 @@ add_resource_record(struct dns_header *header, unsigned int nameoffset,
         switch (*format) {
 #ifdef HAVE_IPV6
         case '6':
+            if (!CHECK_LEN(header, p, plen, IN6ADDRSZ)) {
+                return 0;
+            }
             sval = va_arg(ap, char *);
             memcpy(p, sval, IN6ADDRSZ);
             p += IN6ADDRSZ;
             break;
 #endif
 
         case '4':
+            if (!CHECK_LEN(header, p, plen, INADDRSZ)) {
+                return 0;
+            }
             sval = va_arg(ap, char *);
             memcpy(p, sval, INADDRSZ);
             p += INADDRSZ;
             break;
 
         case 's':
+            if (!CHECK_LEN(header, p, plen, 2)) {
+                return 0;
+            }
             usval = va_arg(ap, int);
             PUTSHORT(usval, p);
             break;
 
         case 'l':
+            if (!CHECK_LEN(header, p, plen, 4)) {
+                return 0;
+            }
             lval = va_arg(ap, long);
             PUTLONG(lval, p);
             break;
@@ -338,6 +353,9 @@ add_resource_record(struct dns_header *header, unsigned int nameoffset,
         case 't':
             usval = va_arg(ap, int);
             sval  = va_arg(ap, char *);
+            if (!CHECK_LEN(header, p, plen, usval)) {
+                return 0;
+            }
             if (usval != 0) {
                 memcpy(p, sval, usval);
             }
@@ -350,6 +368,9 @@ add_resource_record(struct dns_header *header, unsigned int nameoffset,
             if (usval > 255) {
                 usval = 255;
             }
+            if (!CHECK_LEN(header, p, plen, (1 + usval))) {
+                return 0;
+            }
             *p++ = (unsigned char) usval;
             memcpy(p, sval, usval);
             p += usval;
@@ -359,6 +380,9 @@ add_resource_record(struct dns_header *header, unsigned int nameoffset,
     va_end(ap); /* clean up variable argument pointer */
 
     j = p - sav - 2;
+    if (!CHECK_LEN(header, sav, plen, 2)) {
+        return 0;
+    }
     PUTSHORT(j, sav); /* Now, store real RDLength */
 
     *pp = p;

rfc1035.h

@@ -13,7 +13,7 @@ int extract_name(struct dns_header *header, size_t plen, unsigned char **pp,
                  char *name, int isExtract, int extrabytes);
 
 int add_resource_record(struct dns_header *header, unsigned int nameoffset,
-                        unsigned char **pp, unsigned long ttl,
+                        size_t plen, unsigned char **pp, unsigned long ttl,
                         unsigned int *offset, unsigned short type,
                         unsigned short class, char *format, ...);
 

tcp_request.c

@@ -88,10 +88,10 @@ tcp_listener_kill_oldest_request(struct context *c)
  */
 static int
 self_serve_cert_file(struct context *c, struct dns_header *header,
-                     size_t dns_query_len, TCPRequest *tcp_request)
+                     size_t dns_query_len, size_t max_len, TCPRequest *tcp_request)
 {
     uint8_t dns_query_len_buf[2];
-    if (dnscrypt_self_serve_cert_file(c, header, &dns_query_len) == 0) {
+    if (dnscrypt_self_serve_cert_file(c, header, &dns_query_len, max_len) == 0) {
         dns_query_len_buf[0] = (dns_query_len >> 8) & 0xff;
         dns_query_len_buf[1] = dns_query_len & 0xff;
         if (bufferevent_write(tcp_request->client_proxy_bev,
@@ -206,7 +206,7 @@ client_proxy_read_cb(struct bufferevent *const client_proxy_bev,
     struct dns_header *header = (struct dns_header *)dns_query;
     // self serve signed certificate for provider name?
     if (!tcp_request->is_dnscrypted) {
-        if (self_serve_cert_file(c, header, dns_query_len, tcp_request) == 0)
+        if (self_serve_cert_file(c, header, dns_query_len, sizeof_dns_query, tcp_request) == 0)
             return;
         if (!c->allow_not_dnscrypted) {
             logger(LOG_DEBUG, "Unauthenticated query received over TCP");

udp_request.c

@@ -234,9 +234,9 @@ timeout_timer_cb(evutil_socket_t timeout_timer_handle, short ev_flags,
  */
 static int
 self_serve_cert_file(struct context *c, struct dns_header *header,
-                     size_t dns_query_len, UDPRequest *udp_request)
+                     size_t dns_query_len, size_t max_len, UDPRequest *udp_request)
 {
-    if (dnscrypt_self_serve_cert_file(c, header, &dns_query_len) == 0) {
+    if (dnscrypt_self_serve_cert_file(c, header, &dns_query_len, max_len) == 0) {
             SendtoWithRetryCtx retry_ctx = {
                 .udp_request = udp_request,
                 .handle = udp_request->client_proxy_handle,
@@ -333,7 +333,7 @@ client_to_proxy_cb(evutil_socket_t client_proxy_handle, short ev_flags,
 
     // self serve signed certificate for provider name?
     if (!udp_request->is_dnscrypted) {
-        if (self_serve_cert_file(c, header, dns_query_len, udp_request) == 0)
+        if (self_serve_cert_file(c, header, dns_query_len, sizeof_dns_query, udp_request) == 0)
             return;
         if (!c->allow_not_dnscrypted) {
             logger(LOG_DEBUG, "Unauthenticated query received over UDP");