From bb8321c637502aa16d1418f4803d4b83f6ef2ba1 Mon Sep 17 00:00:00 2001 From: Michal Nowak Date: Wed, 20 May 2026 17:58:41 +0000 Subject: [PATCH] Validate nsec3hash arguments instead of relying on atoi() 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 (cherry picked from commit e13302a6bc9b196564b5e7afe703fae24311ceeb) --- bin/tools/nsec3hash.c | 32 ++++++++++++++++++++------------ 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/bin/tools/nsec3hash.c b/bin/tools/nsec3hash.c index 1228be96f0..ae0b703c38 100644 --- a/bin/tools/nsec3hash.c +++ b/bin/tools/nsec3hash.c @@ -23,6 +23,7 @@ #include #include #include +#include #include #include #include @@ -82,10 +83,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[] = "-"; @@ -104,17 +105,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);