buffer overflow code audit.

git-svn-id: file:///svn/unbound/trunk@680 be551aaa-1e26-0410-a405-d3ace91eadb9

diff --git a/daemon/unbound.c b/daemon/unbound.c
index e696fffbd04aa58244ed50f682f50b23ffa585d1..5cfd162e23dd560b6b8b2ab53f978ab46547a822 100644 (file)
--- a/daemon/unbound.c
+++ b/daemon/unbound.c
@@ -183,6 +183,7 @@ readpid (const char* file)
                return -1;
        }
 
+       pidbuf[sizeof(pidbuf)-1] = 0;
        pid = (pid_t)strtol(pidbuf, &t, 10);
        
        if (*t && *t != '\n') {

diff --git a/doc/Changelog b/doc/Changelog
index 638db8be6efaeb0748f5a65e0075c9e560b273ab..229789f8177de94869ae63ba550a78697569a295 100644 (file)
--- a/doc/Changelog
+++ b/doc/Changelog
@@ -2,6 +2,14 @@
        - no malloc in log_hex.
        - assertions around system calls.
        - protect against gethostname without ending zero.
+       - ntop output is null terminated by unbound.
+       - pidfile content null termination
+       - various snprintf use sizeof(stringbuf) instead of fixed constant.
+       - changed loopdetect % 8 with & 0x7 since % can become negative for
+         weird negative input and particular interpretation of integer math.
+       - dname_pkt_copy checks length of result, to protect result buffers.
+         prints an error, this should not happen. Bad strings should have
+         been rejected earlier in the program.
 
 15 October 2007: Wouter
        - nicer warning.

diff --git a/services/listen_dnsport.c b/services/listen_dnsport.c
index 48f7c5e1a21192dea7dddee583c57f1fec4b205d..960a8f37696eb326ff6ec1d51a32d817d4c30c59 100644 (file)
--- a/services/listen_dnsport.c
+++ b/services/listen_dnsport.c
@@ -74,6 +74,7 @@ verbose_print_addr(struct addrinfo *addr)
                        (socklen_t)sizeof(buf)) == 0) {
                        strncpy(buf, "(null)", sizeof(buf));
                }
+               buf[sizeof(buf)-1] = 0;
                verbose(VERB_ALGO, "creating %s%s socket %s %d", 
                        addr->ai_socktype==SOCK_DGRAM?"udp":
                        addr->ai_socktype==SOCK_STREAM?"tcp":"otherproto",

diff --git a/util/data/dname.c b/util/data/dname.c
index e0826a29f45a6f6a08c37ef6c8e3f0507011a1a3..cfa9196138b69d937cffc5d4428b37c235caba43 100644 (file)
--- a/util/data/dname.c
+++ b/util/data/dname.c
@@ -152,8 +152,8 @@ loopcheck(uint8_t loop[], size_t pos)
        const uint8_t bits[8] = {0x1, 0x2, 0x4, 0x8, 0x10, 0x20, 0x40, 0x80};
        uint8_t ret;
        log_assert(pos < MAX_COMPRESS_POS);
-       ret = loop[ pos / 8 ] & bits[ pos % 8 ];
-       loop[ pos / 8 ] |= bits[ pos % 8 ];     
+       ret = loop[ pos / 8 ] & bits[ pos & 0x7 ];
+       loop[ pos / 8 ] |= bits[ pos & 0x7 ];   
        return ret;
 }
 
@@ -305,6 +305,7 @@ dname_pkt_hash(ldns_buffer* pkt, uint8_t* dname, hashvalue_t h)
 void dname_pkt_copy(ldns_buffer* pkt, uint8_t* to, uint8_t* dname)
 {
        /* copy over the dname and decompress it at the same time */
+       size_t len = 0;
        uint8_t lablen;
        lablen = *dname++;
        while(lablen) {
@@ -315,6 +316,12 @@ void dname_pkt_copy(ldns_buffer* pkt, uint8_t* to, uint8_t* dname)
                        continue;
                }
                log_assert(lablen <= LDNS_MAX_LABELLEN);
+               len += (size_t)lablen+1;
+               if(len >= LDNS_MAX_DOMAINLEN) {
+                       *to = 0; /* end the result prematurely */
+                       log_err("bad dname in dname_pkt_copy");
+                       return;
+               }
                *to++ = lablen;
                memmove(to, dname, lablen);
                dname += lablen;

diff --git a/util/net_help.c b/util/net_help.c
index 7eadca4fd78611fe58c2aea456dab5db73e101b5..0fa6a44c1464041242e952525a3ffb0f257e83c0 100644 (file)
--- a/util/net_help.c
+++ b/util/net_help.c
@@ -164,6 +164,7 @@ log_addr(const char* str, struct sockaddr_storage* addr, socklen_t addrlen)
         if(inet_ntop(af, sinaddr, dest, (socklen_t)sizeof(dest)) == 0) {
                 strncpy(dest, "(inet_ntop error)", sizeof(dest));
         }
+       dest[sizeof(dest)-1] = 0;
         port = ntohs(((struct sockaddr_in*)addr)->sin_port);
         verbose(VERB_DETAIL, "%s %s %s %d (len %d)",
                 str, family, dest, (int)port, (int)addrlen);
@@ -244,14 +245,14 @@ log_nametypeclass(enum verbosity_value v, const char* str, uint8_t* name,
        else if(ldns_rr_descript(type) && ldns_rr_descript(type)->_name)
                ts = ldns_rr_descript(type)->_name;
        else {
-               snprintf(t, 12, "TYPE%d", (int)type);
+               snprintf(t, sizeof(t), "TYPE%d", (int)type);
                ts = t;
        }
        if(ldns_lookup_by_id(ldns_rr_classes, (int)dclass) &&
                ldns_lookup_by_id(ldns_rr_classes, (int)dclass)->name)
                cs = ldns_lookup_by_id(ldns_rr_classes, (int)dclass)->name;
        else {
-               snprintf(c, 12, "CLASS%d", (int)dclass);
+               snprintf(c, sizeof(c), "CLASS%d", (int)dclass);
                cs = c;
        }
        log_info("%s <%s %s %s>", str, buf, ts, cs);
@@ -279,6 +280,7 @@ void log_name_addr(enum verbosity_value v, const char* str, uint8_t* zone,
         if(inet_ntop(af, sinaddr, dest, (socklen_t)sizeof(dest)) == 0) {
                 strncpy(dest, "(inet_ntop error)", sizeof(dest));
         }
+       dest[sizeof(dest)-1] = 0;
         port = ntohs(((struct sockaddr_in*)addr)->sin_port);
        dname_str(zone, namebuf);
        if(af != AF_INET && af != AF_INET6)

diff --git a/util/region-allocator.c b/util/region-allocator.c
index 6cf4dafebfd2e76701460a89958b459bee9d4416..23dac4d6b3554bccec646104ef318d41eab7860a 100644 (file)
--- a/util/region-allocator.c
+++ b/util/region-allocator.c
@@ -510,7 +510,7 @@ region_log_stats(region_type *region)
 {
        char buf[10240], *str=buf;
        int len=0;
-       snprintf(str, 10240, "%lu objects (%lu small/%lu large), %lu bytes allocated (%lu wasted) in %lu chunks, %lu cleanups, %lu in recyclebin%n",
+       snprintf(str, sizeof(buf), "%lu objects (%lu small/%lu large), %lu bytes allocated (%lu wasted) in %lu chunks, %lu cleanups, %lu in recyclebin%n",
                (unsigned long) (region->small_objects + region->large_objects),
                (unsigned long) region->small_objects,
                (unsigned long) region->large_objects,
@@ -532,7 +532,7 @@ region_log_stats(region_type *region)
                                el = el->next;
                        }
                        if(i%ALIGNMENT == 0 && i!=0) {
-                               snprintf(str, 10240, " %lu%n", 
+                               snprintf(str, sizeof(buf)-(str-buf), " %lu%n", 
                                        (unsigned long)count, &len);
                                str+=len;
                        }