Re: [Dnsmasq-discuss] reproducible segmentation fault - bisected!

2017-08-29 Thread Kevin Darbyshire-Bryant



On 28/08/17 17:27, Christian Kujau wrote:

On Mon, 28 Aug 2017, Christian Kujau wrote:

On Mon, 28 Aug 2017, Kevin Darbyshire-Bryant wrote:

My workaround is to only call memset if the difference between buffer begin
and buffer limit is bigger than the query length, thus it retains Simon's
intent of clearing memory most of the time but avoids the SIGSEGV trampling.


Thanks, with your patch dnsmasq doesn't crash anymore when receiving odd
EDNS packets from dnseval.


Here is a fix rather than my sticking plaster workaround.  My workaround 
patch would actually allow dnsmasq to generate invalid replies, this 
actually *fixes* the problem!


Cheers,

Kevin
>From 38af9b1ac3242a4128e88069c495024caa565f0e Mon Sep 17 00:00:00 2001
From: Kevin Darbyshire-Bryant 
Date: Tue, 29 Aug 2017 12:35:40 +0100
Subject: [PATCH] forward.c: fix CVE-2017-13704

Fix SIGSEGV in rfc1035.c answer_request() line 1228 where memset()
is called with header & limit pointing at the same address and thus
tries to clear memory from before the buffer begins.

answer_request() is called with an invalid edns packet size provided by
the client.  Ensure the udp_size provided by the client is bounded by
512 and configured maximum as per RFC 6891 6.2.3 "Values lower than 512
MUST be treated as equal to 512"

The client that exposed the problem provided a payload udp size of 0.

Signed-off-by: Kevin Darbyshire-Bryant 
---
 src/forward.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/forward.c b/src/forward.c
index f22556a..62c5a5a 100644
--- a/src/forward.c
+++ b/src/forward.c
@@ -1408,6 +1408,8 @@ void receive_query(struct listener *listen, time_t now)
 	 defaults to 512 */
   if (udp_size > daemon->edns_pktsz)
 	udp_size = daemon->edns_pktsz;
+  if (udp_size < 512)
+	udp_size = 512; /* RFC 6891 6.2.3 */
 }
 
 #ifdef HAVE_AUTH
-- 
2.7.4

___
Dnsmasq-discuss mailing list
Dnsmasq-discuss@lists.thekelleys.org.uk
http://lists.thekelleys.org.uk/mailman/listinfo/dnsmasq-discuss


Re: [Dnsmasq-discuss] reproducible segmentation fault - bisected!

2017-08-29 Thread Kevin Darbyshire-Bryant
I've a *much* better fix for this.  Will submit once I've collected 
someone from the station!


Mad busy life,

Kevin

On 28/08/17 17:27, Christian Kujau wrote:

On Mon, 28 Aug 2017, Christian Kujau wrote:

On Mon, 28 Aug 2017, Kevin Darbyshire-Bryant wrote:

My workaround is to only call memset if the difference between buffer begin
and buffer limit is bigger than the query length, thus it retains Simon's
intent of clearing memory most of the time but avoids the SIGSEGV trampling.


Thanks, with your patch dnsmasq doesn't crash anymore when receiving odd
EDNS packets from dnseval.

And thanks for requesting the CVE - I thought about this too, as the bug
constitutes some kind of DoS issue, but since nobody else complained, I
suspected it to be some variation of PEBKAC on my part :)


Oh, I believe it was Juan Manuel requesting the CVE - thanks!

C.



___
Dnsmasq-discuss mailing list
Dnsmasq-discuss@lists.thekelleys.org.uk
http://lists.thekelleys.org.uk/mailman/listinfo/dnsmasq-discuss