Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix crashing induced by Zonemaster::LDNS::RR::NSEC3::salt() method #177

Merged
merged 2 commits into from
Nov 21, 2023
Merged

Fix crashing induced by Zonemaster::LDNS::RR::NSEC3::salt() method #177

merged 2 commits into from
Nov 21, 2023

Commits on Nov 20, 2023

  1. Make Zonemaster::LDNS::RR::NSEC3::salt work 

    The salt() method in the Zonemaster::LDNS::RR::NSEC3 module never worked
and often caused the Perl interpreter to crash. This commit fixes many
long-standing issues with the affected code.

Firstly, the root cause of the crash is a double free resulting from the
inappropriate use of ldns_rdf_deep_free() in the code. The
ldns_nsec3_salt() function returns a pointer to a ldns_rdf structure
which is just a window into the salt field, not a copy of the data. So
calling ldns_rdf_deep_free() on that ldns_rdf object causes a part of
the original resource record structure to be freed instead. This then
results in a double free when the memory for the resource record object
is deallocated. Calling ldns_rdf_free() instead fixes the crashing.

Secondly, the method doesn’t quite return the salt: it actually returns
a string containing the salt preceded by its length byte. This is
surprising, not as documented and unlikely to be useful. This problem is
fixed by rewriting the entire function so as to return the salt, all the
salt and nothing but the salt.

Thirdly, the method was also insufficiently covered by unit tests. Tests
were added, first to help reproduce the crashes, but also to cover the
case of an NSEC3 with non-empty salt.

Finally, the method returns undef if the salt is empty. Not only is that
documented nowhere, but the choice of doing so is questionable. This
commit changes the behavior somewhat in this case: if the salt is empty,
an empty string is returned instead; the method only returns undef if
there was a problem accessing the salt field.
    @marc-vanderwal
    marc-vanderwal committed Nov 20, 2023
    Configuration menu
    Copy the full SHA
    e9f3bff View commit details
    Browse the repository at this point in the history

  2. Remove newlines from base64-encoded data

    @marc-vanderwal
    marc-vanderwal committed Nov 20, 2023
    Configuration menu
    Copy the full SHA
    b8f44fe View commit details
    Browse the repository at this point in the history