Closed Willy-Tan closed 6 years ago
thanks for the report :)
I added a regression test (which decodes the frame and afterwards expects error
to produce something sensible (and esp. not crash).
the fix I applied is if 450 byte are not sufficient to encode out only the first question. max_udp_size
is my sanity guard to not act as an amplifier (which maybe should be tweaked to be a ratio between incoming and outgoing packet) -- given that I don't support any frames with multiple questions, I don't think I need to send the entire question back (please let me know if there's a requirement in some RFC to always send back the entire question section).
Sending the entire question back isn't a compulsory requirement but RFC5452, a Proposed Standard RFC, advises that the question section in the answer should match the question section from the query to help against spoofed packets. There may be some resolvers implementing this RFC, so that may be an issue, although I don't really know if there are many packets asking for multiples questions in a single query.
I will continue fuzzing in that direction, there may be other cases in which it could happen. The parsing function looks really solid though, I haven't found any bugs here 🤔
After trying to fuzz the primary server from the example folder with big inputs, I found some packets that could crash
main.native
. In this example, the culprit is a query of size 3451 that consists of many questions (https://pastebin.com/tnV0JUbR for a hexadecimal and byte representation of that packet) :TL;DR : The packet is faulty according to the uDNS primary server handle function because the question section contains more than one question. The server tries to reply with an answer containing the same question section (I think it must be done for security purposes), but it creates a buffer with a shorter length, that's why that error is raised and crashes the application.
Now for a more detailed explanation (took me a really long time to figure out !). If I understood correctly :
ERR [dns_server] partial frame (length 4096)
), so the packet goes throughUDns_server.Primary.handle
is called at one pointUDns_server.Primary.handle_inner
is called, which calls itselfUDns_server.handle_frame
UDns_server.handle_query
UDns_server.handle_query
checks the question section and sees that there is more than one question, therefore it returns the rcodeFormErr
toUDns_server.handle_inner
UDns_server.handle_inner
, seeing that it got an rcode error, callsUDns_server.err
, which callsDns_packet.error
to create a reply with the same content but with the rcode set toFormErr
Dns_packet.error
creates a buffer of sizeDns_packet.max_reply_udp
(= 450 !!) and tries to copy the initial packet into that buffer throughDns_packet.encode_query
Dns_packet.encode_query
callsList.fold_left
usingDns_packet.encode_question
as the folding function, which shifts the offset value in the reply bufferinvalid bounds
is raised.Maybe a solution would be to rise
max_udp_size
to 4096 ?