[Dnsmasq-discuss] [BUG] [PATCH] Segmentation fault in src/forward.c

Dominik DL6ER dl6er at dl6er.de
Thu Sep 16 14:00:26 UTC 2021


Hi again,

after applying my patch from my previous mail, dnsmasq stopped
crashing. However, this only started to reveal abnormal and
partially conflicting behavior in certain configurations.

For instance, using the new attached configuration does make
dnsmasq behave strangely. The log contains what we want

> dnsmasq: using nameserver 8.8.8.8#53
> dnsmasq: using nameserver 192.168.2.1#53 for domain
2.168.192.in-addr.arpa 
> dnsmasq: using nameserver 192.168.2.1#53 for domain fritz.box 
> dnsmasq: using standard nameservers for bo.net

However, the query A bo.net is NOT sent to 8.8.8.8 but REFUSED:

> dnsmasq: 1 127.0.0.1/59299 query[A] bo.net from 127.0.0.1
> dnsmasq: 1 127.0.0.1/59299 config error is REFUSED (EDE:
network error)

Ay other domain works fine. Looking around in the source code, I
see a few more places where it looks like struct server* is
accessed in more locations in ways that are not correct because
they are in fact serv_addr4, serv_addr6, or serv_local and hence
shorter than "struct server" and we're actually accessing some
memory from some other element. This just never leads to a crash
because we are not trying to deference the random data. It is
still nonsense.

The safest and most future-safe fix would be to always allocate
the full "struct server", but I can see that this leads to a lot
of memory overhead for "--address".

Another idea might be to change struct server to look like:

struct server {
  u16 flags, domain_len;
  char *domain;
  struct server *next;
  union extra {
    /* flags does not have SERV_LITERAL_ADDRESS */
    struct serv_extra *serv;
    /* flags has SERV_LITERAL_ADDRESS and SERV_4ADDR */
    struct serv_addr4 *addr4;
    /* flags has SERV_LITERAL_ADDRESS and SERV_6ADDR */
    struct serv_addr6 *addr6;
  } extra;
}

struct serv_extra {
  int serial, arrayposn;
  int last_server;
  union mysockaddr addr, source_addr;
  // [... and the rest of them...]
}

struct serv_addr4 {
  struct in_addr addr;
}

struct serv_addr6 {
  struct in6_addr addr;
}


Here, "sizeof(struct server)" would always be the same and we
allocate the correct member of "union details" as needed (this
can clearly be derived from the flags). It'd at least ensure we
would not have a struct server* that is only partially allocated.

As much could be done by search+replace, e.g., "->sfd" to "-
>extra->serv->sfd", the amount of work sound not too dramatic.

However, as this is a design decision and as you may want to do
it differently altogether, I'm not attaching a patch here.

Best,
Dominik
-------------- next part --------------
log-queries=extra
port=5000

no-resolv
server=8.8.8.8

rev-server=192.168.2.0/24,192.168.2.1
server=/fritz.box/192.168.2.1

# Block AAAA bo.net, all other types should go to 8.8.8.8
server=/bo.net/#
address=/bo.net/::


More information about the Dnsmasq-discuss mailing list