Skip to content

Conversation

@Kolkman
Copy link

@Kolkman Kolkman commented Oct 25, 2022

When trying to debug some code with 'dig' I noticed that the code would return a SERVFAIL (or other configured error code) because of the EDNS in the additional section. I rewrote the code to be able to answer queries with EDNS on. Makes troubleshooting with dig much easier.

Also, reply packet handling was such that a SERFVAIL would return a few extra bytes in the packet (easy to observe with wireshark).

The code would return an A resource record also when the QTYPE was not A. I fixed that for other QTYPE SERVAIL is now returned.

Finally the domain name parsing might not terminate if the question domain name in the packet is crafted to have no nul-byte terminating it, potentially leading to DOS.

All this was fixed with rewriting some of the internal and private functions.

Fix: reply with malformed SERVFAIL packets, i.e. with an additional couple of bytes in the packet. Now: clean DNS packet boundary.
Fix: code would reply with A record also for other QTYPES. Now: will answer only A type queries,
Fix: code would return SERVAIL when EDNS was set on request (and copy all its content as extraneous packet content). Now: cleanly ignores EDNS0
Fix: Added check on parsing of QNAME to prevent potential problems with corrupt QNAMES

@benpeart
Copy link

benpeart commented Nov 1, 2022

I'm happy to see these fixes given how frequently this library is used by other libraries. I have one request, please update the version in library.json with this change. I've noticed that platform.io isn't picking up the correct version with the ESP32 fixes guestisp made back in May 2022 as those changes weren't properly versioned either.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants