]> 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)
committerMark Andrews <marka@isc.org>
Wed, 5 Feb 2020 01:41:13 +0000 (01:41 +0000)
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.

lib/dns/resolver.c

index c56d320445f4f48afe8ba3ee3aacb473e77794de..c36a70c8a90ac7133673192af938245faa65dfbd 100644 (file)
@@ -7220,19 +7220,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);
        pbuf = isc_mem_get(mctx, nsid_len + 1);
 
        /* 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];
        }
@@ -7240,11 +7242,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';
 
@@ -7254,10 +7253,8 @@ log_nsid(isc_buffer_t *opt, size_t nsid_len, resquery_t *query,
                      DNS_LOGMODULE_RESOLVER, level,
                      "received NSID %s (\"%s\") from %s", buf, pbuf, addrbuf);
 
-       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