Stop using the namebuff buffer to read config file
lines. This reduced in size in
014e909f78 and caused
regressions. We now resize the buffer as needed,
so there is no practical limit.
Use the same code to remove the same limit when reading
/etc/ethers and /etc/resolv.conf. Also when copying output
from the lease-change script into the log.
Thanks to Kevin Darbyshire-Bryant for finding the regresssion.
This is the stable release fix for CVE-2026-2291
Dnsmasq recieves and sends domain names within DNS packets in RFC-1035
format, a string of counted labels terminated by a zero length label.
Internally to dnsmasq, domain names are represented as zero-terminated C strings
with the labels seperated by '.' characters, like the normal presentation
format for domain names.
The RFC-1035 limit for a domain name in wire format is 255 octets, and
when converted to presentation format, this makes a string of
maximum 253 characters. However dnsmasq needs to be able to
represent any 8-bit character within a label, and this causes
problems with /000 (which would terminate the c-string early) and
'.' (which would terminate a label early). To avoid this, dnsmasq
escapes both of these characters, which means that they take two bytes
in the string. The domain name with must charaters in it has four labels
and the 255 octet wire length limits these four labels to a total of
250 characters. The other five octets are the four label length bytes
and the zero length byte terminator. If all 250 characters need escaping
that makes the maximum length of the dnsmasq internal format 503
characters (250 escaped characters in labels, amd three dots between
four labels) Since this format is zero-terminated c-string, buffers
have to be allocated as 504 bytes.
This arrangement grew in a somewhat ad-hoc manner and early dnsmasq
didn't do the escaping trick, and didn't really differentiate between
the lengths of the wire format and the presentation format, since
they were very similar. It also, for reasons lost in time, inherited
a defintion for MAXDNAME from early BIND headers, set at 1025 bytes.
This value was used as the buffer size for both wire and presentation
formats.
This patch makes everything consistent. It declares two constants
MAXDNAME 255 /* max size of wire format domain name */
MAXDNAMESTR 503 /* max size of internal format created from a 255 octet wire format name */
All internal name buffers are allocated as MAXDNAMESTR+1 bytes,
to hold a maximum size internal format name and zero termination.
Names parsed out of incoming packets are rejected if their
wire format is greater than 255 bytes, which ensures that
their internal representation cannot exceed 503 characters.
Names inserted into outgoing packets are failed similarly if
they exceed 255 bytes. (this would in theory be possible
for a legal internal-representaion name if it has at least
one unescaped character in a label.
When t == 0 every conditional branch is skipped and buf is never
written, leaving it without a NUL terminator. The caller in
dhcp-common.c log_opts() does `p += strlen(p)` immediately after,
scanning uninitialised stack memory until it finds a stray zero byte.
Reachable from the network: a local attacker can craft a DHCP packet
with any time-typed option (e.g. option 51) set to 0x00000000,
triggering this path via the option-logging code. Impact is limited
to stack bytes appearing in syslog output (information disclosure),
not reachable for code execution. Fix by zeroing buf[0] first so
the buffer is always well-formed.
This bug does not look like it could be a remotely triggerable DoS
vector as strlen(buf) on an uninitialised heap buffer will almost
certainly find a NUL byte in adjacent heap metadata before
hitting an unmapped page.
Signed-off-by: Dominik <dl6er@dl6er.de>
rand64() declared its own `static int outleft`, shadowing the global
one while all three functions share the same out[] SURF output buffer.
When rand64() called surf() to refill out[], the global outleft still
indexed into the now-overwritten buffer, so subsequent rand32() or
rand16() calls could return the same words rand64() had already
consumed. Drop the local static so all three functions coordinate
through the same counter.
Security: rand64() is used to generate DHCPv6 temporary address
interface IDs, which are visible on the local network. Because of the
shared buffer issue, an observer could reconstruct the out[]
words consumed by rand64() and predict the next rand32() outputs.
rand32() drives the 0x20 case-scrambling bitmap used to harden DNS
queries against cache-poisoning, so this leaks PRNG state across the
DHCPv6 and DNS code paths. I'd still say the overall severity is rather
low as local network access required, and 0x20 encoding is really a
defence-in-depth and not some kind of essential security.
Signed-off-by: Dominik <dl6er@dl6er.de>
memset(p+old, 0, new-old) was missing the sizeof(unsigned char *)
multiplier. p is unsigned char**, so each slot is pointer-sized (8
instead of 1 byte on 64-bit), yet only new-old bytes were zeroed,
leaving newly allocated DNSSEC workspace pointer slots with garbage values.
Signed-off-by: Dominik <dl6er@dl6er.de>
dhcp-option=66,
sets the servername to nothing, and causes a segfault at DHCP
transaction time. There may be other ways to provoke this, for
instance by using an empty filename.
The patch make safe_strncpy() even safer, by handing a NULL src argument.
Thanks to Jeff Allen for spotting and reporting this.
If iovcnt is exhausted and the first vector element's operation
is satisfied, the while loop will read past the end of the
iovec array. This triggers the address sanitizer and leads to
undefined program state. Avoid reading too far.
In the DNS TCP code, there are a couple of places where
we have a buffer containing a message which we need to
send via TCP. The DNS protocol is that this is sent as
<16-bit length in network order><message>
Making two write calls, one for the length and one
for the message causes the TCP stack to send two packets,
one for each. A single packet containing both is
preferable from a performance POV.
Implement a scatter-gather version of our read_write()
wrapper and use it where necessary to send TCP DNS messages.
The presence of wrong RRSIG RRs in replies causes DNSSEC
validation to fail even when the RRs do not require validation
because the zone is unsigned.
This patch solves the problem and tidies up the logic.
Included is some fixes for hostname_issubdomain() which suffered
some confusions about argument order :) I've clarified that and
checked every to the function to make sure they are using the
correct argument order.
Note that, at the time of this commit, Google DNS appears to have
the same bug, so if you're using 8.8.8.8 or friends as upstream,
resolving the broken zones (eg rivcoed.org) will still fail.
Thanks to Petr Menšík for the bug report.
This fixes a problem introduced in 8a5fe8ce6b
On BSD, fdescfs is normally mounted at /dev/fd. However
if it is NOT mounted, devfs creates a directory at /dev/fd
which contains (only) the file descriptors 0,1 and 2.
Under these conditions, opendir() will succeed, and
if we proceed we will fail to close extant
file descriptors which should be closed.
Check that there is a filesystem mounted at /dev/fd
by checking that the device changes between /dev/fd
and /dev. If if doesn't, fall back to the dumb path.
Thanks to Roman Bogorodskiy for spotting the problem
and helping with diagnosis.
I misread the man page for socket(7) and TCP timeouts.
A timeout generates a -1 return and EAGAIN errno, NOT a short read.
Short reads are legit, and aborting when they are seen creates
hard-to-reproduce errors.
I am attaching an incremental git-am ready patch to go on top your Git HEAD,
to fix all sorts of issues and make this conforming C99 with default
options set,
and fix another load of warnings you receive when setting the compiler
to pick the nits,
-pedantic-errors -std=c99 (or c11, c18, c2x).
It changes many void * to uint8_t * to make the "increment by bytes"
explicit.
You can't do:
void *foo;
// ...
foo += 2.
Timeouts for TCP connections to non-responive servers are very long.
This in not appropriate for DNS connections.
Set timeouts for connection setup, sending data and recieving data.
The timeouts for connection setup and sending data are set at 5 seconds.
For recieving the reply this is doubled, to take into account the
time for usptream to actually get the answer.
Thanks to Petr Menšík for pointing out this problem, and finding a better
and more portable solution than the one in place heretofore.
In generalising the RR filter code, the Dbus methods
controlling filtering A and AAAA records
got severely broken. This, and the previous commit,
fixes things.
1) It's expected to fail to bind a new source port when they
are scarce, suppress warning in log in this case.
2) Optimse bind_local when max_port - min_port is small. There's no
randomness in this case, so we try all possible source ports
rather than poking at random ones for an arbitrary number of tries.
3) In allocate_rfd() handle the case that all available source ports
are already open. In this case we need to pick an existing
socket/port to use, such that it has a different port from any we
already hold. This gives the required property that the set of ports
utilised by any given query is set by --port-limit and we don't
re-use any until we have port-limit different ones.
This gives dnsmasq the ability to originate retries for upstream DNS
queries itself, rather than relying on the downstream client. This is
most useful when doing DNSSEC over unreliable upstream network. It
comes with some cost in memory usage and network bandwidth.
Move few patters with whine_malloc, if (successful) copy+free, to a new
whine_realloc. It should do the same thing, but with a help from OS it
can avoid unnecessary copy and free if allocation of more data after
current data is possible.
Added few setting remanining space to 0, because realloc does not use
calloc like whine_malloc does. There is no advantage of zeroing what we
will immediately overwrite. Zero only remaining space.
The domain-match rewrite didn't take into account
that domain names are case-insensitive, so things like
--address=/Example.com/.....
didn't work correctly.
Transitional encoding accepts every emoticon you can think about.
Because setlocale were not enabled before, IDN 2003 input was not
accepted by dnsmasq. It makes no sense therefore to maintain backward
compatibility. Accept only proper encoded unicode names and reject
random unicode characters.
Signed-off-by: Petr Menšík <pemensik@redhat.com>
The dnsmasq_time() function, in the case of HAVE_BROKEN_RTC, was calling
times() to read the number of ticks "elapsed since an arbitrary point in
the past" and then dividing that by sysconf(_SC_CLK_TCK) to compute the
number of seconds elapsed since that arbitrary instant. This works fine
until the number of ticks exceeds 2^31, beyond which time the function
would begin erroneously returning negative times. On my system this
happens after approximately 248 days of uptime. A symptom is that
dnsmasq no longer populates the resolver cache with DHCP-derived names
at startup, as the inserted cache entries immediately expire due to
having negative expiration times that cause is_expired() to return true
when called with now==0.
This commit replaces the archaic implementation of dnsmasq_time() with a
call to the POSIX-standardized clock_gettime(CLOCK_MONOTONIC), thereby
eliminating the need to convert manually from ticks to seconds. The new
implementation will yield correct results until the system uptime
exceeds approximately 68 years.
Signed-off-by: Matt Whitlock <dnsmasq@mattwhitlock.name>
Optimization that only runs IDN processing if it would alter the domain
name (non-ascii or uppercase characters).
This patch has conributions from Petr Menšík.
Calls to libidn on names without with only a-z A-Z - _ 0-9
have no effect, but are slow. This change elides those calls.
Patch inspire by analysis and an earlier patch from
Gustaf Ullberg <gustaf.ullberg@gmail.com>
CVE-2021-3448 applies.
It's possible to specify the source address or interface to be
used when contacting upstream nameservers: server=8.8.8.8@1.2.3.4
or server=8.8.8.8@1.2.3.4#66 or server=8.8.8.8@eth0, and all of
these have, until now, used a single socket, bound to a fixed
port. This was originally done to allow an error (non-existent
interface, or non-local address) to be detected at start-up. This
means that any upstream servers specified in such a way don't use
random source ports, and are more susceptible to cache-poisoning
attacks.
We now use random ports where possible, even when the
source is specified, so server=8.8.8.8@1.2.3.4 or
server=8.8.8.8@eth0 will use random source
ports. server=8.8.8.8@1.2.3.4#66 or any use of --query-port will
use the explicitly configured port, and should only be done with
understanding of the security implications.
Note that this change changes non-existing interface, or non-local
source address errors from fatal to run-time. The error will be
logged and communiction with the server not possible.
Dnsmasq needs to close all the file descriptors it inherits, for security
reasons. This is traditionally done by calling close() on every possible
file descriptor (most of which won't be open.) On big servers where
"every possible file descriptor" is a rather large set, this gets
rather slow, so we use the /proc/<pid>/fd directory to get a list
of the fds which are acually open.
This only works on Linux. On other platforms, and on Linux systems
without a /proc filesystem, we fall back to the old way.
Simon Kelley