Comments to CVE-2023-5680
Comments to CVE-2023-5680:
Description: When reviewing the fix for CVE-2023-5680 due to the crash we
reported separately, we've noticed many other suspicious points in its implemen
-tation. Though these are based on code inspection and we haven't checked whether
the issue is real or it can cause any practical problem like a crash, we're
deeply concerned about the overall quality of this implementation, and would
like to suggest ISC revisiting it, perhaps fundamentally.
The issues we've noticed are as follows (there may be more):
-
it looks like a longer prefix match in ->old_ecs_root will not be found if
a shorter prefix match is found in ->ecs_root. When using two address prefix
trees, we ought to search both and use the longest prefix match, with ->ecs_root
in preference if both have equal prefix lengths. -
On a related note, it seems possible that copying (moving) data in old_ecs_root
to ecs_root can result in separate rdatasetheaders at the top level for the same record type. -
unlikely to be a big deal in practice, but this code in clean_iptree_nodedata()
probably doesn't do what it appears to intend; it results in cleaning up to 12 -
as a meta issue, we're afraid the introduction of old_ecs_root and incremental
cleaning needs a lot more tests, especially low level unit tests, given its comp
-lexity. For example, if the last point is indeed an oversight, it could have
been caught by a unit test easily.
See also #4587