[Dnsmasq-discuss] dnsmasq mishandles some cases when bad dns response packet is received

[Geert Stappers]

On Wed, Nov 23, 2022 at 10:56:20AM +0800, zhangjiangyu via Dnsmasq-discuss wrote:
> On 23/11/2022 07:21, Simon via Dnsmasq-discuss wrote:
> > 
> > The main argument for this seems to be a security one: the client may 
> > not handle a malformed packet, and a suitably crafted malformed packet 
> > may compromise the client with a buffer overflow or similar. If the 
> > client is sending all requests via dnsmasq, then dnsmasq should detect 
> > and eliminate malformed packets from upstream rather than sending them 
> > to the client as this protects the client from compromise.
> > 
> > If we have a client which is vulnerable to malformed packets then the 
> > solution is to fix the client, not put it behind dnsmasq. Putting the 
> > client behind dnsmasq and relying on dnsmasq to intercept malformed 
> > packets is not fix since and attacker may be able to send malformed 
> > packets direct to the client anyway or the configuration may change such 
> > that the client talks directly to other servers which the attacker may 
> > have control over. Using dnsmasq to filter malformed packets from the 
> > client is at best fragile and at worst doesn't work. It also transfers 
> > the responsibility to handle untrusted data from the client to dnsmasq, 
> > which is the wrong approach.

So true


> > Dnsmasq has to accept malformed packets from the internet without 
> > crashing or being compromised, and, as far as anyone knows, it does. 

And I'm very happy with that.


> > Because of the fairly unique architecture of dnsmasq, altering it to 
> > only return known correctly formed packets is a large change which adds 
> > to the code footprint, increases that chance that something which is in 
> > fact correct but encodes DNS data which dnsmasq doesn't understand will 
> > be rejected by mistake, and can't guarantee to catch all malformed 
> > packets anyway.
> > 
> > I can see the argument for flagging packets which dnsmasq detects are 
> > malformed as part of it's code to extract data for caching, but I don't 
> > think attempting to detect all malformed packets is required. the only 
> > reason to do that is to protect vulnerable clients, and that problem is 
> > fixed by fixing the clients.
> > 
> 
> Ok, Now I understand that the fairly unique architecture of dnsmasq
> cannot verify all malformed packets.
> But I think there should be a basic detection of the packets. sure,
> dnsmasq does not need to judge the legality of each record resource
> data.
> 
> like this basic record structure:
> 
> |                                    1  1  1  1  1  1
> |      0  1  2  3  4  5  6  7  8  9  0  1  2  3  4  5
> |    +--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+
> |    |                                               |
> |    /                                               /
> |    /                      NAME                     /
> |    |                                               |
> |    +--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+
> |    |                      TYPE                     |
> |    +--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+
> |    |                     CLASS                     |
> |    +--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+
> |    |                      TTL                      |
> |    |                                               |
> |    +--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+
> |    |                   RDLENGTH                    |
> |    +--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--|
> |    /                     RDATA                     /
> |    /                                               /
> |    +--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+
> 
> The structure of the record is stationary, and the rfc stipulates that
> the value of the class has a certain scope and meaning, so the clear
> regulation of the dns rfc should be checked and necessary. But for
> the first bug, there is really no check, I think this should be fixed.
 
I think that many many things[1] should be fixed.
Just thinking about it will not bring the desired improvements.

Please move away from "the first bug".
Use a name like "request1-response1-combination"
Describe what the outcome should be. In the begin of this email thread
there was "RC 2" and "RC 3", if I recall correct. Later on there was
SERVFAIL. I hope that SERVFAIL is either "RC 2" or "RC 3". My point is
that I'm lost in what/which solution is wished for.


> |The following CLASS mnemonics and values are defined:
> |
> |IN              1 the Internet
> |
> |CS              2 the CSNET class (Obsolete - used only for examples in
> |                some obsolete RFCs)
> |
> |CH              3 the CHAOS class
> |
> |HS              4 Hesiod [Dyer 87]


Yes, please elaborate.

Long version of the "Yes, please elaborate":
  Acknowlegde on 'There are only four CLASS mnemonics: IN, CS, CH and HS'
  Want are you expecting from the people you are addressing?


 
> Thank you very much for your reply.
> 
> Thanks,
> P1n9

Groeten
Geert Stappers


[1] on world scale, so also things way beyond dnsmasq
-- 
Silence is hard to parse