From 5abdee9004f118b2c1301229418f93de7626e66f Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Fri, 4 Mar 2022 09:37:39 +1100 Subject: [PATCH 1/4] Prevent arithmetic overflow of 'i' in master.c:generate the value of 'i' in generate could overflow when adding 'step' to it in the 'for' loop. Use an unsigned int for 'i' which will give an additional bit and prevent the overflow. The inputs are both less than 2^31 and and the result will be less than 2^32-1. --- lib/dns/master.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/dns/master.c b/lib/dns/master.c index 4f0dce775f..7399729161 100644 --- a/lib/dns/master.c +++ b/lib/dns/master.c @@ -790,7 +790,8 @@ generate(dns_loadctx_t *lctx, char *range, char *lhs, char *gtype, char *rhs, isc_buffer_t target; isc_result_t result; isc_textregion_t r; - int i, n, start, stop, step = 0; + int n, start, stop, step = 0; + unsigned int i; dns_incctx_t *ictx; char dummy[2]; @@ -845,7 +846,7 @@ generate(dns_loadctx_t *lctx, char *range, char *lhs, char *gtype, char *rhs, goto insist_cleanup; } - for (i = start; i <= stop; i += step) { + for (i = start; i <= (unsigned int)stop; i += step) { result = genname(lhs, i, lhsbuf, DNS_MASTER_LHS); if (result != ISC_R_SUCCESS) { goto error_cleanup; From 9039aad0f87e0942437702c448afc5a178f34e83 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Fri, 4 Mar 2022 09:51:22 +1100 Subject: [PATCH 2/4] Add test case for issue-45178 --- .../generate-counter-overflow.db | Bin 0 -> 47 bytes 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 fuzz/dns_master_load.in/generate-counter-overflow.db diff --git a/fuzz/dns_master_load.in/generate-counter-overflow.db b/fuzz/dns_master_load.in/generate-counter-overflow.db new file mode 100644 index 0000000000000000000000000000000000000000..590c0d5c04ffa71ae409ccc08c12b2a1e66e723d GIT binary patch literal 47 zcmY#!clC1(atv|RFtpS)vd}j&G%+`^Fg7zW< Date: Fri, 4 Mar 2022 15:13:25 -0800 Subject: [PATCH 3/4] update shell syntax clean up the shell syntax in the checkzone test prior to adding a new test. --- bin/tests/system/checkzone/tests.sh | 62 ++++++++++++++--------------- 1 file changed, 31 insertions(+), 31 deletions(-) diff --git a/bin/tests/system/checkzone/tests.sh b/bin/tests/system/checkzone/tests.sh index 442dbba6bc..c8f05925b9 100644 --- a/bin/tests/system/checkzone/tests.sh +++ b/bin/tests/system/checkzone/tests.sh @@ -29,9 +29,9 @@ do $CHECKZONE -i local example $db > test.out.$n 2>&1 || ret=1 ;; esac - n=`expr $n + 1` + n=$((n+1)) if [ $ret != 0 ]; then echo_i "failed"; fi - status=`expr $status + $ret` + status=$((status+ret)) done for db in zones/bad*.db @@ -47,9 +47,9 @@ do ;; esac test $v = 1 || ret=1 - n=`expr $n + 1` + n=$((n+1)) if [ $ret != 0 ]; then echo_i "failed"; fi - status=`expr $status + $ret` + status=$((status+ret)) done echo_i "checking with journal file ($n)" @@ -57,16 +57,16 @@ ret=0 $CHECKZONE -D -o test.orig.db test zones/test1.db > /dev/null 2>&1 || ret=1 $CHECKZONE -D -o test.changed.db test zones/test2.db > /dev/null 2>&1 || ret=1 $MAKEJOURNAL test test.orig.db test.changed.db test.orig.db.jnl 2>&1 || ret=1 -jlines=`$JOURNALPRINT test.orig.db.jnl | wc -l` +jlines=$($JOURNALPRINT test.orig.db.jnl | wc -l) [ $jlines = 3 ] || ret=1 $CHECKZONE -D -j -o test.out1.db test test.orig.db > /dev/null 2>&1 || ret=1 cmp -s test.changed.db test.out1.db || ret=1 mv -f test.orig.db.jnl test.journal $CHECKZONE -D -J test.journal -o test.out2.db test test.orig.db > /dev/null 2>&1 || ret=1 cmp -s test.changed.db test.out2.db || ret=1 -n=`expr $n + 1` +n=$((n+1)) if [ $ret != 0 ]; then echo_i "failed"; fi -status=`expr $status + $ret` +status=$((status+ret)) echo_i "checking with spf warnings ($n)" ret=0 @@ -78,57 +78,57 @@ grep "'example' found type SPF" test.out1.$n > /dev/null && ret=1 grep "'x.example' found type SPF" test.out2.$n > /dev/null && ret=1 grep "'y.example' found type SPF" test.out2.$n > /dev/null && ret=1 grep "'example' found type SPF" test.out2.$n > /dev/null && ret=1 -n=`expr $n + 1` +n=$((n+1)) if [ $ret != 0 ]; then echo_i "failed"; fi -status=`expr $status + $ret` +status=$((status+ret)) echo_i "checking with max ttl (text) ($n)" ret=0 $CHECKZONE -l 300 example zones/good1.db > test.out1.$n 2>&1 && ret=1 $CHECKZONE -l 600 example zones/good1.db > test.out2.$n 2>&1 || ret=1 -n=`expr $n + 1` +n=$((n+1)) if [ $ret != 0 ]; then echo_i "failed"; fi -status=`expr $status + $ret` +status=$((status+ret)) echo_i "checking with max ttl (raw) ($n)" ret=0 $CHECKZONE -f raw -l 300 example good1.db.raw > test.out1.$n 2>&1 && ret=1 $CHECKZONE -f raw -l 600 example good1.db.raw > test.out2.$n 2>&1 || ret=1 -n=`expr $n + 1` +n=$((n+1)) if [ $ret != 0 ]; then echo_i "failed"; fi -status=`expr $status + $ret` +status=$((status+ret)) echo_i "checking for no 'inherited owner' warning on '\$INCLUDE file' with no new \$ORIGIN ($n)" ret=0 $CHECKZONE example zones/nowarn.inherited.owner.db > test.out1.$n 2>&1 || ret=1 grep "inherited.owner" test.out1.$n > /dev/null && ret=1 -n=`expr $n + 1` +n=$((n+1)) if [ $ret != 0 ]; then echo_i "failed"; fi -status=`expr $status + $ret` +status=$((status+ret)) echo_i "checking for 'inherited owner' warning on '\$ORIGIN + \$INCLUDE file' ($n)" ret=0 $CHECKZONE example zones/warn.inherit.origin.db > test.out1.$n 2>&1 || ret=1 grep "inherited.owner" test.out1.$n > /dev/null || ret=1 -n=`expr $n + 1` +n=$((n+1)) if [ $ret != 0 ]; then echo_i "failed"; fi -status=`expr $status + $ret` +status=$((status+ret)) echo_i "checking for 'inherited owner' warning on '\$INCLUDE file origin' ($n)" ret=0 $CHECKZONE example zones/warn.inherited.owner.db > test.out1.$n 2>&1 || ret=1 grep "inherited.owner" test.out1.$n > /dev/null || ret=1 -n=`expr $n + 1` +n=$((n+1)) if [ $ret != 0 ]; then echo_i "failed"; fi -status=`expr $status + $ret` +status=$((status+ret)) echo_i "checking that raw zone with bad class is handled ($n)" ret=0 $CHECKZONE -f raw example zones/bad-badclass.raw > test.out.$n 2>&1 && ret=1 grep "failed: bad class" test.out.$n >/dev/null || ret=1 -n=`expr $n + 1` +n=$((n+1)) if [ $ret != 0 ]; then echo_i "failed"; fi -status=`expr $status + $ret` +status=$((status+ret)) echo_i "checking that expirations that loop using serial arithmetic are handled ($n)" ret=0 @@ -155,45 +155,45 @@ test $ret -eq 1 || $CHECKZONE $q dyn.example.net zones/crashzone.db || ret=1 test $ret -eq 1 || $CHECKZONE $q dyn.example.net zones/crashzone.db || ret=1 test $ret -eq 1 || $CHECKZONE $q dyn.example.net zones/crashzone.db || ret=1 test $ret -eq 1 || $CHECKZONE $q dyn.example.net zones/crashzone.db || ret=1 -n=`expr $n + 1` +n=$((n+1)) if [ $ret != 0 ]; then echo_i "failed"; fi -status=`expr $status + $ret` +status=$((status+ret)) echo_i "checking that nameserver below DNAME is reported even with occulted address record present ($n)" ret=0 $CHECKZONE example.com zones/ns-address-below-dname.db > test.out.$n 2>&1 && ret=1 grep "is below a DNAME" test.out.$n >/dev/null || ret=1 -n=`expr $n + 1` +n=$((n+1)) if [ $ret != 0 ]; then echo_i "failed"; fi -status=`expr $status + $ret` +status=$((status+ret)) echo_i "checking that delegating nameserver below DNAME is reported even with occulted address record present ($n)" ret=0 $CHECKZONE example.com zones/delegating-ns-address-below-dname.db > test.out.$n 2>&1 || ret=1 grep "is below a DNAME" test.out.$n >/dev/null || ret=1 -n=`expr $n + 1` +n=$((n+1)) if [ $ret != 0 ]; then echo_i "failed"; fi -status=`expr $status + $ret` +status=$((status+ret)) -n=`expr $n + 1` +n=$((n+1)) echo_i "checking that named-compilezone works when reading input from stdin ($n)" ret=0 # Step 1: take raw input from stdin and convert it to text/relative format. # Last argument "-" is optional, but it says more explicitly that we're reading from stdin. cat zones/zone1.db | ./named-compilezone -f text -F text -s relative \ -o zones/zone1_stdin.txt zone1.com - > /dev/null || ret=1 -status=`expr $status + $ret` +status=$((status+ret)) ret=0 # Step 2: take raw input from file and convert it to text format. ./named-compilezone -f text -F text -s relative -o zones/zone1_file.txt \ zone1.com zones/zone1.db > /dev/null || ret=1 -status=`expr $status + $ret` +status=$((status+ret)) ret=0 # Step 3: Ensure that output conversion from stdin is the same as the output conversion from a file. diff zones/zone1_file.txt zones/zone1_stdin.txt >/dev/null 2>&1 || ret=1 -status=`expr $status + $ret` +status=$((status+ret)) if [ $ret != 0 ]; then echo_i "failed"; fi From bd814b79d4a87faf80e306d705a6a9cc0ae08c11 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Fri, 4 Mar 2022 15:19:52 -0800 Subject: [PATCH 4/4] add a system test for $GENERATE with an integer overflow the line "$GENERATE 19-28/2147483645 $ CNAME x" should generate a single CNAME with the owner "19.example.com", but prior to the overflow bug it generated several CNAMEs, half of them with large negative values. we now test for the bugfix by using "named-checkzone -D" and grepping for a single CNAME in the output. --- bin/tests/system/checkzone/tests.sh | 9 +++++++++ .../system/checkzone/zones/generate-overflow.db | 17 +++++++++++++++++ 2 files changed, 26 insertions(+) create mode 100644 bin/tests/system/checkzone/zones/generate-overflow.db diff --git a/bin/tests/system/checkzone/tests.sh b/bin/tests/system/checkzone/tests.sh index c8f05925b9..a63e221ea9 100644 --- a/bin/tests/system/checkzone/tests.sh +++ b/bin/tests/system/checkzone/tests.sh @@ -193,9 +193,18 @@ status=$((status+ret)) ret=0 # Step 3: Ensure that output conversion from stdin is the same as the output conversion from a file. diff zones/zone1_file.txt zones/zone1_stdin.txt >/dev/null 2>&1 || ret=1 +if [ $ret != 0 ]; then echo_i "failed"; fi status=$((status+ret)) +n=$((n+1)) +ret=0 +echo_i "checking integer overflow is prevented in \$GENERATE ($n)" +$CHECKZONE -D example.com zones/generate-overflow.db > test.out.$n 2>&1 || ret=1 +lines=$(grep -c CNAME test.out.$n) +echo $lines +[ "$lines" -eq 1 ] || ret=1 if [ $ret != 0 ]; then echo_i "failed"; fi +status=$((status+ret)) echo_i "exit status: $status" [ $status -eq 0 ] || exit 1 diff --git a/bin/tests/system/checkzone/zones/generate-overflow.db b/bin/tests/system/checkzone/zones/generate-overflow.db new file mode 100644 index 0000000000..c16b517481 --- /dev/null +++ b/bin/tests/system/checkzone/zones/generate-overflow.db @@ -0,0 +1,17 @@ +; Copyright (C) Internet Systems Consortium, Inc. ("ISC") +; +; SPDX-License-Identifier: MPL-2.0 +; +; This Source Code Form is subject to the terms of the Mozilla Public +; License, v. 2.0. If a copy of the MPL was not distributed with this +; file, you can obtain one at https://mozilla.org/MPL/2.0/. +; +; See the COPYRIGHT file distributed with this work for additional +; information regarding copyright ownership. + +$TTL 600 +@ SOA ns hostmaster 2011012708 3600 1200 604800 1200 + NS ns +ns A 192.0.2.1 + +$GENERATE 19-28/2147483645 $ CNAME x