Hello list, The --rebind-domain-ok option is broken in v2.86 and on master in the following ways:
* In v2.85, --stop-dns-rebind --rebind-domain-ok=test.me would only allow
"test.me" and subdomains of "test.me" to return private addresses to the
user. A query for localtest.me, which is known to return 127.0.0.1, is
blocked as expected.
In v2.86, the --rebind-domain-ok feature is implemented with a simple suffix
comparison, which means that --stop-dns-rebind --rebind-domain-ok=test.me
fails to block the response of "127.0.0.1" for "localtest.me" because
"test.me" is a suffix of "localtest.me".
Here is a reproducible example:
v2.85$ src/dnsmasq -C /dev/null -a 127.0.0.1 -p 5353 -S 1.1.1.1 -qd
--no-resolv --stop-dns-rebind --rebind-domain-ok=test.me
dnsmasq: started, version 2.85 cachesize 150
dnsmasq: compile time options: IPv6 GNU-getopt no-DBus no-UBus no-i18n
no-IDN DHCP DHCPv6 no-Lua TFTP no-conntrack ipset auth no-cryptohash no-DNSSEC
loop-detect inotify dumpfile
dnsmasq: using nameserver 1.1.1.1#53
dnsmasq: read /etc/hosts - 1 addresses
dnsmasq: query[A] localtest.me from 127.0.0.1
dnsmasq: forwarded localtest.me to 1.1.1.1
dnsmasq: possible DNS-rebind attack detected: localtest.me
master$ src/dnsmasq -C /dev/null -a 127.0.0.1 -p 5353 -S 1.1.1.1 -qd
--no-resolv --stop-dns-rebind --rebind-domain-ok=test.me
dnsmasq: started, version 2.87test4-4-gc0409fa cachesize 150
dnsmasq: compile time options: IPv6 GNU-getopt no-DBus no-UBus no-i18n
no-IDN DHCP DHCPv6 no-Lua TFTP no-conntrack ipset no-nftset auth no-cryptohash
no-DNSSEC loop-detect inotify dumpfile
dnsmasq: using nameserver 1.1.1.1#53
dnsmasq: read /etc/hosts - 1 addresses
dnsmasq: query[A] localtest.me from 127.0.0.1
dnsmasq: forwarded localtest.me to 1.1.1.1
dnsmasq: reply localtest.me is 127.0.0.1
* In v2.85, --stop-dns-rebind --rebind-domain-ok=// means "stop potential DNS
rebinding attacks, but allow private responses for dotless domains", which
mirrors the special meaning of // in the --server option.
In v2.86, --stop-dns-rebind --rebind-domain-ok=// crashes dnsmasq during
resolution.
v2.85$ src/dnsmasq -C /dev/null -a 127.0.0.1 -p 5353 -S 192.168.0.1 -qd
--no-resolv --stop-dns-rebind --rebind-domain-ok=//
dnsmasq: started, version 2.85 cachesize 150
dnsmasq: compile time options: IPv6 GNU-getopt no-DBus no-UBus no-i18n
no-IDN DHCP DHCPv6 no-Lua TFTP no-conntrack ipset auth no-cryptohash no-DNSSEC
loop-detect inotify dumpfile
dnsmasq: using nameserver 1.1.1.1#53
dnsmasq: read /etc/hosts - 1 addresses
dnsmasq: query[A] brother-laser-printer from 127.0.0.1
dnsmasq: forwarded brother-laser-printer to 192.168.0.1
dnsmasq: reply brother-laser-printer is 192.168.0.50
master$ src/dnsmasq -C /dev/null -a 127.0.0.1 -p 5353 -S 192.168.0.1 -qd
--no-resolv --stop-dns-rebind --rebind-domain-ok=//
dnsmasq: started, version 2.87test4-2-g9560658 cachesize 150
dnsmasq: compile time options: IPv6 GNU-getopt no-DBus no-UBus no-i18n
no-IDN DHCP DHCPv6 no-Lua TFTP no-conntrack ipset no-nftset auth no-cryptohash
no-DNSSEC loop-detect inotify dumpfile
dnsmasq: using nameserver 1.1.1.1#53
dnsmasq: read /etc/hosts - 1 addresses
dnsmasq: query[A] brother-laser-printer from 127.0.0.1
Segmentation fault (core dumped)
Note that the new suffix-matching algorithm of --rebind-domain-ok means that
even if the crash above is fixed, an empty option value effectively negates
--stop-dns-rebind because the empty string is a suffix of all possible
strings.
The attached patches address the issues above and restore the behavior of
--rebind-domain-ok back to the semantics of v2.85. The patches are also
available on Github:
https://github.com/guns/dnsmasq/compare/master...fix-option-rebind-domain-ok
https://github.com/guns/dnsmasq/commit/3abd86eb9e53efeea270767fd251284851d706d4
https://github.com/guns/dnsmasq/commit/4afb5b4ce50a4d3b7f917d2ce83ea1b27dd55db5
From 3abd86eb9e53efeea270767fd251284851d706d4 Mon Sep 17 00:00:00 2001
From: guns <s...@sungpae.com>
Date: Wed, 17 Nov 2021 14:15:35 -0600
Subject: [PATCH 1/2] Correctly return a heap-allocated empty string instead of
NULL
Commit 32e15c3f458c2e8838a9ecf7d478ecb6750516bf added the following
change:
--- a/src/option.c
+++ b/src/option.c
@@ -654,7 +654,7 @@ static char *canonicalise_opt(char *s)
return 0;
if (strlen(s) == 0)
- return "";
+ return opt_string_alloc("");
unhide_metas(s);
if (!(ret = canonicalise(s, &nomem)) && nomem)
Unfortunately, opt_string_alloc(const char *cp) returns NULL when
strlen(cp) == 0, which in turn causes --rebind-domain-ok='' to crash
with SIGSEGV.
---
src/option.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/option.c b/src/option.c
index ac2ba20..9e315a5 100644
--- a/src/option.c
+++ b/src/option.c
@@ -663,7 +663,7 @@ static char *canonicalise_opt(char *s)
return 0;
if (strlen(s) == 0)
- return opt_string_alloc("");
+ return opt_malloc(1); /* Heap-allocated empty string */
unhide_metas(s);
if (!(ret = canonicalise(s, &nomem)) && nomem)
--
2.33.1
From 4afb5b4ce50a4d3b7f917d2ce83ea1b27dd55db5 Mon Sep 17 00:00:00 2001
From: guns <s...@sungpae.com>
Date: Thu, 18 Nov 2021 02:04:02 -0600
Subject: [PATCH 2/2] Fix --rebind-domain-ok domain matching
Commit 32e15c3f458c2e8838a9ecf7d478ecb6750516bf simplified the
implementation of the rebind-domain-ok feature, but unfortunately, it
also changed its semantics.
In v2.85, --stop-dns-rebind --rebind-domain-ok=test.me would only allow
"test.me" and subdomains of "test.me" to return private addresses to
the user. A query for localtest.me, which is known to return 127.0.0.1,
is blocked as expected.
In v2.86, the --rebind-domain-ok feature is implemented as a
simple suffix comparison, which means that --stop-dns-rebind
--rebind-domain-ok=test.me fails to block the response of "127.0.0.1"
for "localtest.me" because "test.me" is a suffix of "localtest.me".
The special meaning of the empty string is also affected by this bug. In
v2.85, --stop-dns-rebind --rebind-domain-ok=// was interpreted to mean
"stop potential DNS rebinding attacks, but allow responses for dotless
domains". This mirrored the special meaning of // in the --server
option.
In contrast, v2.86, --stop-dns-rebind --rebind-domain-ok=// effectively
negates --stop-dns-rebind because the empty string is a suffix of all
possible strings.
This patch restores the semantics of the --rebind-domain-ok option back
to v2.85.
---
src/forward.c | 85 ++++++++++++++++++++++++++++++++++++++++++++++++---
src/option.c | 7 +++++
2 files changed, 88 insertions(+), 4 deletions(-)
diff --git a/src/forward.c b/src/forward.c
index 04635b3..6b2cf6a 100644
--- a/src/forward.c
+++ b/src/forward.c
@@ -149,14 +149,91 @@ static void server_send_log(struct server *server, int fd,
}
#endif
+struct substring {
+ const char *base;
+ size_t offset;
+ size_t len;
+};
+
+static int substrings_are_equal(struct substring *a, struct substring *b)
+{
+ if (a->len == b->len)
+ return strncmp(a->base + a->offset, b->base + b->offset, a->len) == 0;
+ else
+ return 0;
+}
+
+static int rfind_next_domain_part(struct substring *ss)
+{
+ if (ss->offset == 0)
+ return 0; /* No more domain parts */
+
+ ss->len = 0;
+ ss->offset--;
+
+ if (ss->base[ss->offset] == '.')
+ {
+ if (ss->offset == 0)
+ return 0; /* Domain has a leading dot, so there are no more domain
parts */
+ ss->offset--;
+ }
+
+ ss->len++;
+
+ while (ss->offset > 0)
+ {
+ if (ss->base[ss->offset - 1] == '.')
+ break;
+ ss->offset--;
+ ss->len++;
+ }
+
+ return 1;
+}
+
static int domain_no_rebind(char *domain)
{
+ if (daemon->no_rebind == NULL)
+ return 0;
+
struct rebind_domain *rbd;
- size_t tlen, dlen = strlen(domain);
-
+ size_t dlen = strlen(domain);
+
for (rbd = daemon->no_rebind; rbd; rbd = rbd->next)
- if (dlen >= (tlen = strlen(rbd->domain)) && strcmp(rbd->domain,
&domain[dlen - tlen]) == 0)
- return 1;
+ {
+ struct substring domain_ss = {
+ .base = domain,
+ .offset = dlen,
+ .len = 0
+ };
+ struct substring rbd_ss = {
+ .base = rbd->domain,
+ .offset = strlen(rbd->domain),
+ .len = 0
+ };
+ int matches_rebind_domain = 1;
+
+ /* rebind-domain-ok=// means allow "dotless" domains */
+ if (rbd_ss.offset == 0)
+ {
+ if (strchr(domain, '.') == NULL)
+ return 1;
+ else
+ continue;
+ }
+
+ while (rfind_next_domain_part(&rbd_ss))
+ {
+ if (!rfind_next_domain_part(&domain_ss) ||
!substrings_are_equal(&domain_ss, &rbd_ss))
+ {
+ matches_rebind_domain = 0;
+ break;
+ }
+ }
+
+ if (matches_rebind_domain)
+ return 1;
+ }
return 0;
}
diff --git a/src/option.c b/src/option.c
index 9e315a5..4667d3d 100644
--- a/src/option.c
+++ b/src/option.c
@@ -2729,6 +2729,13 @@ static int one_opt(int option, char *arg, char *errstr,
char *gen_err, int comma
comma = split_chr(arg, '/');
new = opt_malloc(sizeof(struct rebind_domain));
new->domain = canonicalise_opt(arg);
+ if (new->domain != 0)
+ {
+ /* Let "example.com." == "example.com" */
+ int dlen = strlen(new->domain);
+ if (dlen > 0 && new->domain[dlen - 1] == '.')
+ new->domain[dlen - 1] = 0;
+ }
new->next = daemon->no_rebind;
daemon->no_rebind = new;
arg = comma;
--
2.33.1
signature.asc
Description: PGP signature
_______________________________________________ Dnsmasq-discuss mailing list Dnsmasq-discuss@lists.thekelleys.org.uk https://lists.thekelleys.org.uk/cgi-bin/mailman/listinfo/dnsmasq-discuss
