]> git.ipfire.org Git - thirdparty/bind9.git/commitdiff
Fix Coverity complaints in the qp-trie tests
authorTony Finch <fanf@isc.org>
Wed, 1 Mar 2023 09:53:35 +0000 (09:53 +0000)
committerTony Finch <fanf@isc.org>
Mon, 3 Apr 2023 15:10:47 +0000 (15:10 +0000)
The main problem was `qp_test_keytoname()` not using `qpkey_bit()`
to do bounds checking.

fuzz/dns_qpkey_name.c
tests/bench/load-names.c
tests/dns/qp_test.c
tests/include/tests/qp.h
tests/libtest/isc.c
tests/libtest/qp.c

index 7cf6f26457c08ccf915626f2eb308a89a20eb515..ed9dd16b3c12a2977c92cbef7af7a1f1009c2f80 100644 (file)
@@ -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));
 
index f621e4fb8bebef8399964f74729b3875b17a0f41..a1473d8694e249bd20eca504aec705b40734eb3c 100644 (file)
@@ -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);
 }
 
index 2ee05a9fdf1cc09471d9d21780e88187ebd94c0a..aceaa344f85dd9756a607218dbfd1854eac31165 100644 (file)
@@ -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));
        }
 }
index 472246ed958624cb60ffee87229ece364f723b53..8e165001aa5c19b919ee245009480b08d4caeb8b 100644 (file)
@@ -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
index bc542a7f1fa2f026355e3c34617c79908e04433d..74e1c51ce766219e7dd98af2fc8d448e000859fb 100644 (file)
@@ -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);
index 0b3e11e001870c36697c273aaaf4337f17ee1477..3866b6548829e11c6a5c9e21b760972324b22697 100644 (file)
@@ -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 {