When --dump-file points to an existing pcap file, dump_init_file()
counts the existing records by reading each pcap record header and
seeking past the payload. The lseek() return value was not checked:
if it fails (e.g. EOVERFLOW on a 32-bit build with a very large
incl_len, or any other seek error) the file position does not advance,
the next read_write() call reads the same header again, and the loop
spins forever — hanging daemon startup.
Fix: break out of the loop on lseek failure. An undercount of
existing records is harmless; a hanging startup is not.
Signed-off-by: Dominik <dl6er@dl6er.de>
strncat(dst, src, n) truncates src to n bytes, but n should be the
remaining space in dst, not the total buffer size. Passing MAXDNAME
as n allowed the combined prefix + address + "." + domain to overflow
the MAXDNAME-byte namebuff if c->domain was close to MAXDNAME chars.
Replace MAXDNAME with MAXDNAME - strlen(name) - 1 at each call site
(lines 178-179 for IPv4, 204 for the IPv6 hex-fragment loop, 208-209
for the IPv6 suffix) so the append is always bounded by actual
remaining capacity.
Security implications: strncat(name, c->domain, MAXDNAME) passes the
full buffer size instead of remaining space. Overflow requires a local
admin to configure a --synth-domain with a domain name > ~960 chars.
The actual exploitable window is narrow: the admin would need a
--synth-domain with a domain name close to 1024 chars (which check_name
permits) combined with a non-trivial prefix or IPv6 address.
Not remotely exploitable.
Signed-off-by: Dominik <dl6er@dl6er.de>
parse_hex() returns -1 on malformed input. Three call sites in
read_leases() used the return value directly as a length argument
without checking for the error sentinel:
opt_len → lease_set_vendorclass() / lease_set_agent_id()
whine_malloc(-1) attempts a ~4 GB allocation, likely
crashing the process.
hw_len → lease_set_hwaddr()
memcpy(lease->hwaddr, hwaddr, (size_t)-1) performs an
~18 EB write, causing an immediate crash.
clid_len → lease_set_hwaddr() (same path as hw_len)
Exploitation requires a corrupted or attacker-controlled lease file;
it is not directly network-triggerable. However, any process that can
write to the lease file (e.g. a compromised helper) can crash or
potentially gain code execution in the dnsmasq process at next startup
or lease-file re-read.
Fix: mirror the existing duid path (which already guards its
parse_hex call) by checking for < 0. For opt_len, skip the record
(continue) to match the "invalid line" behaviour. For hw_len and
clid_len, treat the failure as a zero-length field so the lease is
still created with the correct address.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The EDNS0 option iteration loop read a 2-byte len field from the
packet and used it directly in:
opt.scope_netmask = p[3]; // requires len >= 4
memcmp(p, &opt, len) // reads len bytes from packet
Neither access was guarded against len exceeding the remaining RDATA.
A crafted DNS reply whose OPT RDATA contains a CLIENT_SUBNET option
with an oversized len field would cause memcmp to read past the end of
the packet buffer, leaking heap memory.
Risk: currently the only caller that reaches this branch is
forward.c:process_reply(), which (before the companion fix) passed an
OPT-RR-scoped plen that made skip_name() return NULL first, so the
unsafe code was never reached. With that companion fix applied the
path is live: a rogue upstream server or on-path attacker can craft a
reply with a malformed CLIENT_SUBNET len and trigger the OOB read,
potentially leaking adjacent heap contents.
Fix: break out of the loop if the option body would extend past the
RDATA boundary.
Signed-off-by: Dominik <dl6er@dl6er.de>
In log6_opts(), the OPTION6_STATUS_CODE branch did:
memcpy(daemon->namebuff + len, opt6_ptr(opt, 2), opt6_len(opt)-2);
daemon->namebuff[len + opt6_len(opt) - 2] = 0;
Two bugs:
1. Integer underflow: if opt6_len(opt) < 2, the subtraction wraps to a
huge size_t, making memcpy write gigabytes and crash with SIGSEGV.
2. Heap overflow: no upper-bound check. opt6_len() can return up to
65535, but daemon->namebuff is only (MAXDNAME*2)+1 = 2051 bytes.
A STATUS_CODE payload larger than ~2045 bytes would overflow the
heap buffer.
Exploitability: log6_opts() is only ever called on daemon->outpacket,
the outgoing DHCPv6 reply built by dnsmasq itself — NOT on incoming
client packets. Consequently, these bugs are NOT directly triggerable
by a remote attacker under the current code. The fix is defensive:
put_opt6_short() (which writes STATUS_CODE options in outgoing packets)
always produces opt6_len >= 2, so neither bug fires in practice today.
However, the unchecked arithmetic is a latent hazard that could become
exploitable if future refactoring passes incoming option data through
log6_opts():
- Guard the block with opt6_len(opt) >= 2 to prevent underflow.
- Cap slen to the remaining space in daemon->namebuff.
Signed-off-by: Dominik <dl6er@dl6er.de>
Two decoders in option_string() read beyond the end of the supplied
option-value buffer when a malformed DHCPv6 option is received:
1. OT_RFC1035_NAME: after advancing i past the last label (i = l),
val[i] was read at line 885 to decide whether to append a '.'.
If the label ends exactly at opt_len, i == opt_len and val[i]
is one byte past the option data. Fixed by guarding with
i < opt_len before the read.
2. OT_CSTRING: the loop opened with while(1) and called GETSHORT
(a 2-byte read) before checking that two bytes remained, so a
1- or 0-byte option value caused an immediate 1-byte OOB read.
Additionally, if the length field exceeded the remaining option
data the inner body-copy loop read past the end. Fixed by
changing the loop condition to while(i+2 <= opt_len) and adding
a check that the body fits before copying.
Both reads land inside the received packet buffer (no crash), but
they allow a DHCPv6 client to cause dnsmasq to read unrelated packet
bytes and emit them in syslog output.
Security: OOB reads of up to option-length bytes from a
network-supplied DHCPv6 packet; reachable only with --log-dhcp
enabled; no memory corruption, crash, or sensitive data disclosure
beyond adjacent packet bytes.
Signed-off-by: Dominik <dl6er@dl6er.de>
In option_string(), the RFC1035-name label decoder sets i = l (the
position past the current label) and then reads val[i] to decide
whether to append a '.' separator — but without first checking that
i < opt_len. If the last label ends exactly at the option boundary
(no NUL terminator), i == opt_len and val[i] reads one byte beyond
the option data.
The read lands inside the packet buffer so no crash occurs, but the
out-of-bounds byte (typically the next DHCPv6 option's type field)
could produce a spurious trailing '.' in --log-dhcp output when a
client sends a domain-search/nis-domain option without a trailing NUL.
Security: OOB read of 1 byte from a network-supplied DHCPv6 packet;
reachable only with --log-dhcp enabled; no memory corruption, crash,
or meaningful information disclosure.
Signed-off-by: Dominik <dl6er@dl6er.de>
After copying the 128-byte DHCP file field into dhcp_buff2, the
sentinel was written to index 129 instead of 128. The byte at
index 128 was therefore left holding stale data from the previous
DHCP exchange, so a client that fills the entire file field with
non-NUL bytes would produce a 129-character netid tag whose last
byte is heap garbage. In theory this allows incorrect netid-tag
matching; in practice it requires the client to know the heap
state of a prior transaction and BOOTP use is uncommon.
Security: very-low-severity heap-content exposure that could
influence DHCP option selection via tag mismatch; no memory
corruption or crash.
Signed-off-by: Dominik <dl6er@dl6er.de>
The loop in grab_extradata_lua checked *next != 0 in the loop
condition before the body could test next == end, so when next
reached end the byte *end was read out-of-bounds. The non-Lua
sibling grab_extradata had the correct order (test next == end
before dereferencing). Fix by adopting the same idiom.
Security: a DHCP client can supply a vendor-class option whose
content fills the extradata buffer with no trailing NUL, causing
a 1-byte OOB read in the Lua helper child (already non-root).
Requires HAVE_LUASCRIPT and a configured Lua script. Not feasible
for triggering a crash from remote reliably.
Signed-off-by: Dominik <dl6er@dl6er.de>
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>
The `if (serv == last) return;` was inside the loop
`for (serv = first; serv != last; ...)`, making it dead code — the
loop condition ensures the body never runs when serv == last. Replies
arriving on an unrecognised file descriptor were therefore never
rejected by the fd check. Moving the guard after the loop causes it to
fire when no server socket matches.
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>
If there's no configured server for the parent of the domain
covered by a domain-specific server, assume that said domain is
not signed. This extends the existing logic which makes similar
decisions for replies to DS queries from tghe parent server.
The code sending vendorclass data to the DHCP script
was confused about the format of the vendorclass in
the DHCP packet, resulting in broken data passed
to the packet.
only created after dnsmasq has changed permissions and userid.
This means that the permissions used when creating the watches
are the same as used for accessing watched files, which makes
more sense and avoids odd and confusing error conditions.
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 dnsmasq is configured as an authoritatve server for zone transfer
_only_, (ie no interface name or address in --auth-server)
and secondary auth servers are configured, then
queries for NS RRs at the auth zone will get mangled answers.
This problem doesn't occur in AXFR or SOA queries, only NS queries.
Thanks to Thomas Erbesdobler for finding and analysing this problem.
The patch here is substantially his, with a little but of
collateral code tidying by srk.
Well-known trust anchors can turn up in multiple config files.
Leaving the duplicates makes logging messy and costs some
CPU checking the same DS twice.
I think I've found a regression in dnsmasq 2.92 that breaks PXE boot server
(PXEBS) responses when running in proxy DHCP mode. Fair warning: I'm not
familiar with the dnsmasq codebase and used AI tooling to help trace through
the source and identify the issue, so please take the analysis below with
appropriate skepticism. PXE boot works fine on 2.91
but fails on 2.92 — the client gets the initial proxy DHCPOFFER, but the PXEBS
ACK on port 4011 never reaches it.
My setup is dnsmasq in proxy DHCP mode serving iPXE to Proxmox VMs via their
virtio-net ROM. Here's a stripped-down version of my config:
port=0
enable-tftp
tftp-root=/tftpboot
dhcp-range=172.19.74.0,proxy,255.255.255.0
interface=eno1
bind-interfaces
dhcp-match=set:ipxe,175
pxe-service=tag:ipxe,x86PC,"Network Boot",http://server:8081/boot.ipxe
pxe-service=tag:!ipxe,x86PC,"Network Boot",undionly.kpxe
log-dhcp
The issue seems to be in src/dhcp.c in the response routing logic after
dhcp_reply() returns. In 2.91, the destination selection was an if/else-if
chain:
if (pxe_fd)
{ ... }
else if (mess->giaddr.s_addr && !is_relay_reply)
{ ... }
else if (mess->ciaddr.s_addr)
{ ... }
else
{ ... broadcast to 255.255.255.255:68 ... }
In 2.92, the else between the pxe_fd block and the giaddr/relay check was
removed in commit 4fbe1ad ("Implement RFC-4388 DHCPv4 leasequery") to
accommodate the new is_relay_use_source logic:
if (pxe_fd)
{ ... }
if ((is_relay_use_source || mess->giaddr.s_addr) && !is_relay_reply)
{ ... }
else if (mess->ciaddr.s_addr)
{ ... }
else
{ ... broadcast to 255.255.255.255:68 ... }
For PXEBS responses, dhcp_reply() in rfc2131.c (around line 924-925) does:
mess->yiaddr = mess->ciaddr;
mess->ciaddr.s_addr = 0;
So after dhcp_reply() returns for a PXEBS request, ciaddr is 0, giaddr is 0
(no relay), and is_relay_use_source is 0. In 2.91, the pxe_fd block runs and
the rest of the chain is skipped — dest stays as received from recvmsg, and the
response goes back to the client correctly. In 2.92, the pxe_fd block runs but
then falls through to the standalone if, which is false, so the else block runs
and sets dest to 255.255.255.255 port 68. The client is listening on port 4011
and ignores it.
Here are the relevant dnsmasq logs. With 2.91 (working), I see normal proxy
DHCP and PXE boot server exchanges:
dnsmasq-dhcp: DHCPDISCOVER(eno1) bc:24:11:59:85:90
dnsmasq-dhcp: DHCPOFFER(eno1) 172.19.74.60 bc:24:11:59:85:90
dnsmasq-dhcp: DHCPREQUEST(eno1) 172.19.74.60 bc:24:11:59:85:90
dnsmasq-dhcp: DHCPACK(eno1) 172.19.74.60 bc:24:11:59:85:90
dnsmasq-dhcp: PXE(eno1) bc:24:11:59:85:90 proxy
dnsmasq-dhcp: PXE(eno1) bc:24:11:59:85:90 proxy
dnsmasq-dhcp: PXEBS(eno1) bc:24:11:59:85:90 undionly.kpxe
dnsmasq-dhcp: PXE(eno1) bc:24:11:59:85:90 proxy
dnsmasq-dhcp: PXEBS(eno1) bc:24:11:59:85:90 http://infra1.oneill.net:8081/boot.ipxe
With 2.92 (broken), the DHCPDISCOVER/OFFER/REQUEST/ACK cycle and the proxy
PXE response work, but the PXEBS response never reaches the client — it times
out after repeated attempts. The dnsmasq side shows it sending the response,
but the client keeps retrying:
dnsmasq-dhcp: PXE(eno1) bc:24:11:59:85:90 proxy
dnsmasq-dhcp: PXE(eno1) bc:24:11:59:85:90 proxy
dnsmasq-dhcp: PXEBS(eno1) bc:24:11:59:85:90 undionly.kpxe
dnsmasq-dhcp: PXEBS(eno1) bc:24:11:59:85:90 undionly.kpxe
dnsmasq-dhcp: PXEBS(eno1) bc:24:11:59:85:90 undionly.kpxe
dnsmasq-dhcp: PXEBS(eno1) bc:24:11:59:85:90 undionly.kpxe
I tested by restoring the else keyword and the fix appears to work — 2.92 with
the patch below PXE boots successfully. I believe this change preserves the
leasequery behavior since that path only applies when pxe_fd is false (normal
DHCP handling, not port 4011).
Clearing and reloading the DNS cache involved freeing and re-allocating
many small memory blocks. Doing this frequently with a significant
number of static DNS configuraions or DHCP leases could result
in excessive heap fragmentation and memory use.
This patch extends the pool memory allocation technique used
else where to this memory. On cache clear, the memory is not
freed, but added to a pool of useable but unused data structures
which get re-used to store the new data.
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.
While this won't do harm on systems that do 2's completement,
it triggers the compilers' undefined-behavior sanitizer and
fixes sanitizer error such as the one below (where the 1694... will
vary) and is distracting while debugging.
dnssec.c:1427:12: runtime error: left shift of 1694604366 by 1 places
cannot be represented in type 'int'
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.
This fixes the plethora of 64k buffers that got allocated when doing
DNSSEC over TCP.
By using the UDP buffer is pass the query into tcp_talk() and
allowing tcp_talk to allocate its output buffer one the size
of the reply is known, we only need to allocate as much memory as
is required. The final reply to the TCP query still needs the 64k
buffer because answer_request() and answer_auth() are not capable
of extending their output buffers.
This was functionally correct, but every call
malloced a new buffer and freed the previous one, rather
than only doing that when the buffer needed expansion.
Simon Kelley