]> git.ipfire.org Git - thirdparty/bind9.git/commitdiff
Validate nsec3hash arguments instead of relying on atoi() 12062/head
authorMichal Nowak <mnowak@isc.org>
Wed, 20 May 2026 17:58:41 +0000 (17:58 +0000)
committerMichal Nowak <mnowak@isc.org>
Thu, 21 May 2026 11:34:50 +0000 (13:34 +0200)
The nsec3hash tool parsed its algorithm, flags, and iterations
arguments with atoi(), then range-checked the result. For values
that overflow int during digit-by-digit accumulation, atoi() is
undefined; in practice on musl libc the modular wrap leaves
n == 0, which silently passes the "iterations > 0xffffU" check.
On Alpine Linux this made nsec3hash succeed with iterations
treated as 0 for inputs like 4294967296 (2^32).

The latent bug only surfaced when the recent image rebuild pulled
in Hypothesis 6.152.9 (2026-05-19), which unified the distribution
used for bounded and unbounded integers() strategies. The new
smoother distribution explores the 2^32 boundary on unbounded
ranges like integers(min_value=65536); earlier versions did not
reach there, so test_nsec3hash_too_many_iterations only started
failing on Alpine after the image refresh.

Replace the three atoi() calls with isc_parse_uint8 /
isc_parse_uint16, which uniformly reject overflow, trailing
garbage, leading sign, and non-numeric input across libc
implementations. As a side effect, error messages now include
the offending argument and a specific reason ("out of range" vs
"not a valid number").

Assisted-by: Claude:claude-opus-4-7
bin/tools/nsec3hash.c

index d5735be9efef132430c0a10c397576c2c481984c..20b10ce53f550f8bb7b09222e52e78bd367bdfec 100644 (file)
@@ -23,6 +23,7 @@
 #include <isc/hex.h>
 #include <isc/iterated_hash.h>
 #include <isc/lib.h>
+#include <isc/parseint.h>
 #include <isc/result.h>
 #include <isc/string.h>
 #include <isc/tls.h>
@@ -81,10 +82,10 @@ nsec3hash(nsec3printer *nsec3print, const char *algostr, const char *flagstr,
        unsigned char hash[NSEC3_MAX_HASH_LENGTH];
        unsigned char salt[DNS_NSEC3_SALTSIZE];
        unsigned char text[1024];
-       unsigned int hash_alg;
-       unsigned int flags;
+       uint8_t hash_alg;
+       uint8_t flags;
        unsigned int length;
-       unsigned int iterations;
+       uint16_t iterations;
        unsigned int salt_length;
        const char dash[] = "-";
 
@@ -103,17 +104,24 @@ nsec3hash(nsec3printer *nsec3print, const char *algostr, const char *flagstr,
                        saltstr = dash;
                }
        }
-       hash_alg = atoi(algostr);
-       if (hash_alg > 255U) {
-               fatal("hash algorithm too large");
+       result = isc_parse_uint8(&hash_alg, algostr, 10);
+       if (result != ISC_R_SUCCESS) {
+               fatal("invalid hash algorithm '%s': %s", algostr,
+                     isc_result_totext(result));
        }
-       flags = flagstr == NULL ? 0 : atoi(flagstr);
-       if (flags > 255U) {
-               fatal("flags too large");
+       if (flagstr == NULL) {
+               flags = 0;
+       } else {
+               result = isc_parse_uint8(&flags, flagstr, 10);
+               if (result != ISC_R_SUCCESS) {
+                       fatal("invalid flags '%s': %s", flagstr,
+                             isc_result_totext(result));
+               }
        }
-       iterations = atoi(iterstr);
-       if (iterations > 0xffffU) {
-               fatal("iterations to large");
+       result = isc_parse_uint16(&iterations, iterstr, 10);
+       if (result != ISC_R_SUCCESS) {
+               fatal("invalid iterations '%s': %s", iterstr,
+                     isc_result_totext(result));
        }
 
        name = dns_fixedname_initname(&fixed);