]> git.ipfire.org Git - thirdparty/bind9.git/commitdiff
Fix comparison between type uint16_t and wider type size_t in a loop
authorOndřej Surý <ondrej@isc.org>
Sat, 1 Feb 2020 16:13:45 +0000 (17:13 +0100)
committerOndřej Surý <ondrej@isc.org>
Mon, 10 Feb 2020 10:42:50 +0000 (02:42 -0800)
Found by LGTM.com (see below for description), and while it should not
happen as EDNS OPT RDLEN is uint16_t, the fix is easy.  A little bit
of cleanup is included too.

> In a loop condition, comparison of a value of a narrow type with a value
> of a wide type may result in unexpected behavior if the wider value is
> sufficiently large (or small). This is because the narrower value may
> overflow. This can lead to an infinite loop.

(cherry picked from commit a9bd6f6ea6a4c68e8897c06ff8d3d935521f41e7)

lib/dns/resolver.c

index a543a724eec4dd02e3714b7338d39860b09e5f4a..1ccc04311770cf7715329ffe4091cd46b532b7cf 100644 (file)
@@ -7306,23 +7306,21 @@ log_nsid(isc_buffer_t *opt, size_t nsid_len, resquery_t *query,
 {
        static const char hex[17] = "0123456789abcdef";
        char addrbuf[ISC_SOCKADDR_FORMATSIZE];
-       uint16_t buflen, i;
+       size_t buflen;
        unsigned char *p, *nsid;
        unsigned char *buf = NULL, *pbuf = NULL;
 
+       REQUIRE(nsid_len <= UINT16_MAX);
+
        /* Allocate buffer for storing hex version of the NSID */
-       buflen = (uint16_t)nsid_len * 2 + 1;
+       buflen = nsid_len * 2 + 1;
        buf = isc_mem_get(mctx, buflen);
-       if (buf == NULL)
-               goto cleanup;
        pbuf = isc_mem_get(mctx, nsid_len + 1);
-       if (pbuf == NULL)
-               goto cleanup;
 
        /* Convert to hex */
        p = buf;
        nsid = isc_buffer_current(opt);
-       for (i = 0; i < nsid_len; i++) {
+       for (size_t i = 0; i < nsid_len; i++) {
                *p++ = hex[(nsid[i] >> 4) & 0xf];
                *p++ = hex[nsid[i] & 0xf];
        }
@@ -7330,11 +7328,8 @@ log_nsid(isc_buffer_t *opt, size_t nsid_len, resquery_t *query,
 
        /* Make printable version */
        p = pbuf;
-       for (i = 0; i < nsid_len; i++) {
-               if (isprint(nsid[i]))
-                       *p++ = nsid[i];
-               else
-                       *p++ = '.';
+       for (size_t i = 0; i < nsid_len; i++) {
+               *p++ = isprint(nsid[i]) ? nsid[i] : '.';
        }
        *p = '\0';
 
@@ -7343,11 +7338,9 @@ log_nsid(isc_buffer_t *opt, size_t nsid_len, resquery_t *query,
        isc_log_write(dns_lctx, DNS_LOGCATEGORY_NSID,
                      DNS_LOGMODULE_RESOLVER, level,
                      "received NSID %s (\"%s\") from %s", buf, pbuf, addrbuf);
- cleanup:
-       if (pbuf != NULL)
-               isc_mem_put(mctx, pbuf, nsid_len + 1);
-       if (buf != NULL)
-               isc_mem_put(mctx, buf, buflen);
+
+       isc_mem_put(mctx, pbuf, nsid_len + 1);
+       isc_mem_put(mctx, buf, buflen);
 }
 
 static bool