Queries with opcode set to Notify can crash the resolver

[Willy-Tan]

As the primary server seems to handle fuzz testing without any bug, I tried to fuzz the resolver, and managed to crash it (https://pastebin.com/x3UB87Mf for an example). Here is the resolver log :

2018-07-12 14:11:54 +01:00: INF [application] reacting to (from 127.0.0.1:59757) 0001 query operation Notify rcode NoError flags: : example.com A?
2018-07-12 14:11:54 +01:00: ERR [dns_server] ignoring unsolicited request
2018-07-12 14:11:54 +01:00: ERR [application] answer from authoritative is none, shouldn't happen
Fatal error: exception "Assert_failure resolver/uDns_resolver.ml:239:12"
Raised at file "string.ml", line 118, characters 19-34
Called from file "src0/sexp.ml", line 93, characters 13-47
wt269@eagle:/auto/homes/wt269/OCaml/udns/mirage/examples/resolver$ 

TL;DR : Queries sent with opcode set to Notify are ignored by the primary server that responds with None, to which the resolver raises an error because it doesn't expect a None answer.

For a more detailed debug, which was simpler to find than the other issue :

• The resolver receives a query with the opcode set to Notify and the query flag set to true
• It calls the function UDns_resolver.handle which parses correctly the packet
• The packet is sent by an user so it is considered as a query by the resolver, which is coherent with the flag in the header : UDns_resolver.handle_primary is then called, which calls itself UDns_server.Primary.handle_frame
• As a primary server shouldn't receive a notify+query packet, it ignores it and returns None
• UDns_resovler.handle_primary, seeing that is is a None packet, calls assert false
• There is no try ... with block to catch the error so the resolver crashes.

[hannesm]

thanks for the report - though please keep in mind that uDNS is still under development, and sometimes I use assert false to remind myself to finish code (that's as well the reason why there's no release of this library yet)... as I did in this case where the desired semantics are not obvious to me atm (a git grep assert\ false will discover some more uses)..

[hannesm]

I fixed that, was even able to delete some code

[Willy-Tan]

Oh okay I didn't know about the use of assert false, I was too eager thinking that I found something relevant. I will be more careful next time !

[hannesm]

no worries, and thanks again for the report and your fuzzing work... in the meantime I got rid of the assert false in uDNS (and remarked most with TODO, what I should''ve done from the beginning ;)