ComputerSurge/bind9
3
0
Fork 0
Code Pull Requests 1 Activity

fix dns_qp_lookup() iterator bug

Browse Source 
in some cases it was possible for the iterator to be positioned in the
wrong place by dns_qp_lookup(). previously, when a leaf node was found
which matched the search key at its parent branch's offset point, but
did not match after that point, the code incorrectly assumed the leaf
it had found was a successor to the searched-for name, and stepped the
iterator back to find a predecessor.  however, it was possible for the
non-matching leaf to be the predecessor, in which case stepping the
iterator back was wrong.

(for example: a branch contains "aba" and "abcd", and we are searching
for "abcde". we step down to the twig matching the letter "c" in
position 3. "abcd" is the predecessor of "abcde", so the iterator is
already correctly positioned, but because the twig was an exact match,
we would have moved it back one step to "aba".)

this previously went unnoticed due to a mistake in the qp_test unit
test, which had the wrong expected result for the test case that should
have detected the error. both the code and the test have been fixed.
This commit is contained in:
Evan Hunt
2023-12-03 14:02:29 -08:00
parent 947bc0a432
commit 8612902476
2 changed files with 12 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files

16
lib/dns/qp.c
View File

@@ -2179,18 +2179,22 @@ dns_qp_lookup(dns_qpreadable_t qpr, const dns_name_t *name,
* the predecessor name for a non-exact match.
*
* if 'matched' is true, then the search ended at a leaf.
* if it was not an exact match, then we're now pointing at
* the immediate successor of the searched-for name, and can
* use the qpiter stack we've constructed to step back to
* the predecessor. if it was an exact match, we don't need to
* do anything.
* if it was not an exact match, then we're now pointing
* at either the immediate predecessor or the immediate
* successor of the searched-for name; if successor, we can
* now use the qpiter stack we've constructed to step back to
* the predecessor. if we're pointed at the predecessor
* or it was an exact match, we don't need to do anything.
*
* if 'matched' is false, then the search failed at a branch
* node, and we would already have positioned the iterator
* at the predecessor.
*/
if (getpred && matched) {
if (offset != QPKEY_EQUAL) {
if (offset != QPKEY_EQUAL &&
(offset <= searchlen && offset <= foundlen &&
found[offset] > search[offset]))
{
prevleaf(iter);
}
}

4
tests/dns/qp_test.c
View File

@@ -660,7 +660,7 @@ ISC_RUN_TEST_IMPL(predecessors) {
{ "a.b.c.e.f.", "a.b.c.d.e.", ISC_R_NOTFOUND },
{ "z.y.x.", "a.b.c.d.e.", ISC_R_NOTFOUND },
{ "w.c.d.", "x.k.c.d.", DNS_R_PARTIALMATCH },
{ "z.z.z.z.k.c.d.", "a.b.c.d.", DNS_R_PARTIALMATCH },
{ "z.z.z.z.k.c.d.", "x.k.c.d.", DNS_R_PARTIALMATCH },
{ "w.k.c.d.", "a.b.c.d.", DNS_R_PARTIALMATCH },
{ "d.a.", "e.d.c.b.a.", DNS_R_PARTIALMATCH },
{ "0.b.c.d.e.", "x.k.c.d.", ISC_R_NOTFOUND },
@@ -686,7 +686,7 @@ ISC_RUN_TEST_IMPL(predecessors) {
{ "a.b.c.e.f.", "a.b.c.d.e.", DNS_R_PARTIALMATCH },
{ "z.y.x.", "a.b.c.d.e.", DNS_R_PARTIALMATCH },
{ "w.c.d.", "x.k.c.d.", DNS_R_PARTIALMATCH },
{ "z.z.z.z.k.c.d.", "a.b.c.d.", DNS_R_PARTIALMATCH },
{ "z.z.z.z.k.c.d.", "x.k.c.d.", DNS_R_PARTIALMATCH },
{ "w.k.c.d.", "a.b.c.d.", DNS_R_PARTIALMATCH },
{ "d.a.", "e.d.c.b.a.", DNS_R_PARTIALMATCH },
{ "0.b.c.d.e.", "x.k.c.d.", DNS_R_PARTIALMATCH },