Hey Simon,
today we discovered a segmentation fault in
forward.c:allocate_rfd().
It checks "if (serv->sfd)" before accessing "serv->sfd->fd".
However, we've seen that the value of serv->sfd can be arbitrary
nonsense (such as 0xa1), hence, the access causes a crash here.
The reason for this is that we allocate different amounts of
space in domain-match.c:add_update_server():
> size_t size;
>
> if (flags & SERV_LITERAL_ADDRESS)
> {
> if (flags & SERV_6ADDR)
> size = sizeof(struct serv_addr6);
> else if (flags & SERV_4ADDR)
> size = sizeof(struct serv_addr4);
> else
> size = sizeof(struct serv_local);
> }
> else
> size = sizeof(struct server);
>
> if (!(serv = whine_malloc(size)))
> return 0;
and store the result in daemon->servers as "struct server".
In the crash we've seen allocate_rfd() tried to access the struct
element ->sfd of an "struct serv_addr6" which obviously pointed
deeply into nowhere and sometimes happens to pick up some random
data eventually leading to a crash.
The attached patch tries to fix this by first checking if we are
really dealing with a full "struct server" here. I cannot get
dnsmasq to crash with it.
---
How to reproduce the crash yourself:
1. Use the attached config line.
2. Query "dig A mobile-ixanycast.ftl.netflix.com"
3. Repeat step no. 2 a few times until dnsmasq crashes.
---
Best,
Dominik
server=/netflix.com/#
address=/netflix.com/::
server=/netflix.net/#
address=/netflix.net/::
server=/nflxext.com/#
address=/nflxext.com/::
server=/nflximg.net/#
address=/nflximg.net/::
server=/nflxvideo.net/#
address=/nflxvideo.net/::
server=/swplus-aws.canal-plus.com/#
address=/swplus-aws.canal-plus.com/::
server=/canalplus-bo.net/#
address=/canalplus-bo.net/::
server=/canalplus-cdn.net/#
address=/canalplus-cdn.net/::
server=/apiwaka.azure-api.net/#
address=/apiwaka.azure-api.net/::
server=/wakanim-vid.akamaized.net/#
address=/wakanim-vid.akamaized.net/#
server=/wakanimksm.azurewebsites.net/#
address=/wakanimksm.azurewebsites.net/::
From 716387397b0e1cffc0238c4fc499478da08ade26 Mon Sep 17 00:00:00 2001
From: Dominik DL6ER <dl...@dl6er.de>
Date: Wed, 15 Sep 2021 17:50:43 +0200
Subject: [PATCH] allocate_rfd() tries to access the passed serv as struct
server. However, the passed pointer may actually be a struct serv_addr4,
serv_addr6, or serv_local. All of them are shorter and neither includes
the field ->sfd. Hence, we first have to verify if serv really is of
type struct server which can be done checking its flags.
Signed-off-by: DL6ER <dl...@dl6er.de>
---
src/forward.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/forward.c b/src/forward.c
index f3c38d7..478258e 100644
--- a/src/forward.c
+++ b/src/forward.c
@@ -2229,7 +2229,9 @@ int allocate_rfd(struct randfd_list **fdlp, struct server *serv)
int fd = 0;
/* If server has a pre-allocated fd, use that. */
- if (serv->sfd)
+ /* Only do this when *serv is really a struct server pointer,
+ it could also be serv_addr4, serv_addr6, or serv_local */
+ if (!(serv->flags & SERV_LITERAL_ADDRESS) && serv->sfd)
return serv->sfd->fd;
/* existing suitable random port socket linked to this transaction? */
--
2.25.1
_______________________________________________
Dnsmasq-discuss mailing list
Dnsmasq-discuss@lists.thekelleys.org.uk
https://lists.thekelleys.org.uk/cgi-bin/mailman/listinfo/dnsmasq-discuss
