From 906d434aea19fd950a300cc5d9fad55228ebbd36 Mon Sep 17 00:00:00 2001
From: Tony Finch <fanf@isc.org>
Date: Wed, 1 Mar 2023 09:53:35 +0000
Subject: [PATCH] Fix Coverity complaints in the qp-trie tests

The main problem was `qp_test_keytoname()` not using `qpkey_bit()`
to do bounds checking.
---
 fuzz/dns_qpkey_name.c    |  2 +-
 tests/bench/load-names.c |  2 +-
 tests/dns/qp_test.c      |  2 +-
 tests/include/tests/qp.h |  2 +-
 tests/libtest/isc.c      |  4 ++--
 tests/libtest/qp.c       | 20 ++++++++++++--------
 6 files changed, 18 insertions(+), 14 deletions(-)

diff --git a/fuzz/dns_qpkey_name.c b/fuzz/dns_qpkey_name.c
index 7cf6f26457..ed9dd16b3c 100644
--- a/fuzz/dns_qpkey_name.c
+++ b/fuzz/dns_qpkey_name.c
@@ -56,7 +56,7 @@ LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
 
 	/* verify round-trip conversion of first name */
 	size_t keylen = dns_qpkey_fromname(key, namein);
-	qp_test_keytoname(key, nameout);
+	qp_test_keytoname(key, keylen, nameout);
 
 	assert(dns_name_equal(namein, nameout));
 
diff --git a/tests/bench/load-names.c b/tests/bench/load-names.c
index f621e4fb8b..a1473d8694 100644
--- a/tests/bench/load-names.c
+++ b/tests/bench/load-names.c
@@ -127,7 +127,7 @@ get_ht(void *ht, size_t count, void **pval) {
 static void *
 new_rbt(isc_mem_t *mem) {
 	dns_rbt_t *rbt = NULL;
-	dns_rbt_create(mem, NULL, NULL, &rbt);
+	(void)dns_rbt_create(mem, NULL, NULL, &rbt);
 	return (rbt);
 }
 
diff --git a/tests/dns/qp_test.c b/tests/dns/qp_test.c
index 2ee05a9fdf..aceaa344f8 100644
--- a/tests/dns/qp_test.c
+++ b/tests/dns/qp_test.c
@@ -82,7 +82,7 @@ ISC_RUN_TEST_IMPL(qpkey_name) {
 		assert_memory_equal(testcases[i].key, key, len);
 
 		out = dns_fixedname_initname(&fn2);
-		qp_test_keytoname(key, out);
+		qp_test_keytoname(key, len, out);
 		assert_true(dns_name_equal(in, out));
 	}
 }
diff --git a/tests/include/tests/qp.h b/tests/include/tests/qp.h
index 472246ed95..8e165001aa 100644
--- a/tests/include/tests/qp.h
+++ b/tests/include/tests/qp.h
@@ -34,7 +34,7 @@ qp_test_keytoascii(dns_qpkey_t key, size_t len);
  * functions, this is a complete inverse of dns_qpkey_fromname().
  */
 void
-qp_test_keytoname(const dns_qpkey_t key, dns_name_t *name);
+qp_test_keytoname(const dns_qpkey_t key, size_t len, dns_name_t *name);
 
 /*
  * The maximum height of the trie
diff --git a/tests/libtest/isc.c b/tests/libtest/isc.c
index bc542a7f1f..74e1c51ce7 100644
--- a/tests/libtest/isc.c
+++ b/tests/libtest/isc.c
@@ -75,11 +75,11 @@ setup_loopmgr(void **state ISC_ATTR_UNUSED) {
 	env_workers = getenv("ISC_TASK_WORKERS");
 	if (env_workers != NULL) {
 		workers = atoi(env_workers);
-	} else {
+	}
+	if (workers < 2 || workers > 1000) {
 		/* We always need at least two loops for some of the tests */
 		workers = isc_os_ncpus() + 1;
 	}
-	INSIST(workers != 0);
 
 	isc_loopmgr_create(mctx, workers, &loopmgr);
 	mainloop = isc_loop_main(loopmgr);
diff --git a/tests/libtest/qp.c b/tests/libtest/qp.c
index 0b3e11e001..3866b65488 100644
--- a/tests/libtest/qp.c
+++ b/tests/libtest/qp.c
@@ -62,8 +62,8 @@ qp_test_keytoascii(dns_qpkey_t key, size_t len) {
 }
 
 void
-qp_test_keytoname(const dns_qpkey_t key, dns_name_t *name) {
-	size_t locs[128];
+qp_test_keytoname(const dns_qpkey_t key, size_t keylen, dns_name_t *name) {
+	size_t locs[DNS_NAME_MAXLABELS];
 	size_t loc = 0, opos = 0;
 	size_t offset;
 
@@ -74,14 +74,15 @@ qp_test_keytoname(const dns_qpkey_t key, dns_name_t *name) {
 	isc_buffer_clear(name->buffer);
 
 	/* Scan the key looking for label boundaries */
-	for (offset = 0; offset < 512; offset++) {
+	for (offset = 0; offset <= keylen; offset++) {
 		INSIST(key[offset] >= SHIFT_NOBYTE &&
 		       key[offset] < SHIFT_OFFSET);
-		INSIST(loc < 128);
-		if (key[offset] == SHIFT_NOBYTE) {
-			if (key[offset + 1] == SHIFT_NOBYTE) {
+		INSIST(loc < DNS_NAME_MAXLABELS);
+		if (qpkey_bit(key, keylen, offset) == SHIFT_NOBYTE) {
+			if (qpkey_bit(key, keylen, offset + 1) == SHIFT_NOBYTE)
+			{
 				locs[loc] = offset + 1;
-				break;
+				goto scanned;
 			}
 			locs[loc++] = offset + 1;
 		} else if (offset == 0) {
@@ -89,6 +90,8 @@ qp_test_keytoname(const dns_qpkey_t key, dns_name_t *name) {
 			locs[loc++] = offset;
 		}
 	}
+	UNREACHABLE();
+scanned:
 
 	/*
 	 * In the key the labels are encoded in reverse order, so
@@ -106,7 +109,8 @@ qp_test_keytoname(const dns_qpkey_t key, dns_name_t *name) {
 
 		/* Convert from escaped byte ranges to ASCII */
 		for (offset = locs[loc]; offset < locs[loc + 1] - 1; offset++) {
-			uint8_t byte = dns_qp_byte_for_bit[key[offset]];
+			uint8_t bit = qpkey_bit(key, keylen, offset);
+			uint8_t byte = dns_qp_byte_for_bit[bit];
 			if (qp_common_character(byte)) {
 				isc_buffer_putuint8(name->buffer, byte);
 			} else {