getMACVendor() truncated the address to the 24-bit OUI (XX:YY:ZZ) and matched
that alone. Many OUIs are sub-divided into smaller MA-M (/28) and MA-S / IAB
(/36) assignments whose 24-bit prefix resolves only to "IEEE Registration
Authority" (or to no row at all) - so those devices showed a blank/useless
hardware vendor.
The macvendor table already stores these finer assignments in Wireshark manuf
form (e.g. "34:E1:D1:80/28", "00:1B:C5:00:00/36"), so no database change is
needed: reconstruct the candidate /24, /28 and /36 keys from the queried MAC in
SQL and let the longest match win. Existing /24 keys are matched unchanged, so
this is purely additive - no generator change, no DB rebuild, no transition.
Add a BATS test seeding MA-L/MA-M/MA-S rows that asserts each resolves, that two
devices under the same 24-bit parent resolve independently, and that an unknown
OUI yields no match.
Signed-off-by: RamSet <ramset@gmail.com>
get_rdata() contains another instance of a crash caused by
a false value of RDLEN in a resource-record.
An RR containing name which is longer than the rdlen of the RR can
get get_rdata into a state where it calculates the length of the data
following the name as a negative number which is then promoted
to a very large positive number, and causing SIGSEGV.
This can be triggered by crafted packets from malicous DNS servers,
but only when DNSSEC validation is enabled. The attack packets do not
have to be correctly DNSSEC signed.
Thanks to Tristan G. for spotting this problem.
Also fix error handling for RRs with a fixed size fields in the specification
where the rrsize indicates that there are zero bytes in that position.
Signed-off-by: Dominik <dl6er@dl6er.de>
It turns out that subtracting NULL from a pointer to turn it into
an integer is not a good idea, because subtracting a NULL pointer
is undefined behaviour in C.
The correct way to do this is simply to cast the pointer to
uintptr_t. The only fly in this ointment is that the C standard
does not mandate support for uintptr_t. In practice, I doubt
anyone is running dnsmasq on the long-dead architectures where
this might be a problem.
Thanks to Matthias Andree for the heads-up.
Signed-off-by: Dominik <dl6er@dl6er.de>
If we stop reading a file before getting to the end, get_line_alloc()
will not clean up the memory it allocated. Give get_line_alloc()
the ability to do a forced cleanup by calling it with a NULL file,
and use that ability where we stop reading a file early.
Also clear the state variables when we free the memory, to
pre-empt use-after-free.
Signed-off-by: Dominik <dl6er@dl6er.de>
Assigning the result of getc() to a char makes comparing it to EOF
unreliable, at the very least. You're never too old to make newbie
mistakes.
Thanks to Sebastian Gottschall for spotting this.
Signed-off-by: Dominik <dl6er@dl6er.de>
print_mac() used to output into caller-supplied buffer with no
length checking. The various callers supplied different
length buffers and did their own, ad-hoc, length checking before
calling print_mac(). This was an accident waiting to happen and
it had actually happened in couple of places, with buffer
overflows possible.
print_mac() now returns the address of its own buffer, which
is grown as required.
Thanks to Nicholas Carlini <npc@anthropic.com> for spotting this problem.
Signed-off-by: Dominik <dl6er@dl6er.de>
The current local address range is fd00::/8, but RFC 4193
para 3.1/3.2 reserves fc00::/8 for future use as local addresses,
so add that to the list.
Thanks to Michalis Vasileiadis for spotting this omission.
Signed-off-by: Dominik <dl6er@dl6er.de>
Full credit to Jerry Tom <goonsomeway@gmail.com> for finding this.
When DNSSEC validation falls back to TCP (via a forked child process),
the parent uses a uid-based check to detect whether a frec has been
freed and reused before the child returns. It doesn't, however check for
the more likely scenario which is that the frec has simply been
freed via the timeout path in get_new_frec(). This leads to a
use-after-free on an already freed frec.
Signed-off-by: Dominik <dl6er@dl6er.de>
Also ripple effects for most of its callers.
This fixes an OOB write caused by bad pointer arithmetic
calculating the pointer-to-end-of-packet used before.
Thanks to Donghyeon Jeong for spotting the bug.
Signed-off-by: Dominik <dl6er@dl6er.de>
This fixes an OOB write caused by bad pointer arithmetic
calculating the pointer-to-end-of-packet used before.
Thanks to Donghyeon Jeong for spotting the bug.
Signed-off-by: Dominik <dl6er@dl6er.de>
Before this fix, the code could read a couple of bytes beyond
valid data when an invalid too-short packet was recieved.
Thanks to Sokhna Walo DIAKHATE <sokhnawalodiakhate@esp.sn>
for the bug report.
Signed-off-by: Dominik <dl6er@dl6er.de>
commit 1269f074f86bb959863012063060a3a082d37dc4 broke
hostname_issubdomain() and then accidentally fixed
some of the problems by inverting the argument order
in the new uses of the function it introduced.
All the pre-existing calls to hostname_issubdomain()
were left in a non-working state.
This fixes hostname_issubdomain() back to the state
before 1269f074f86bb959863012063060a3a082d37dc4, adds
some logic to give correct answers when comparing
to the root domain, and gets right the new calls to
hostname_issubdomain() added in
1269f074f86bb959863012063060a3a082d37dc4
Thanks to Jean Thomas for spotting this problem.
Signed-off-by: Dominik <dl6er@dl6er.de>
Stop using the namebuff buffer to read config file
lines. This reduced in size in
014e909f787e808bb35daa546d3f8f3663918de2 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.
Signed-off-by: Dominik <dl6er@dl6er.de>
Thanks to Hugo Martinez Ray for spotting this.
The value of rdlen for an RR can be a lie, allowing the
call to extract_name() at rfc1025.c:952 to advance the value of p1
past the calculated end of the record. The makes the calculation
of bytes remaining in the RR underflow to a huge number and results
in a massive heap OOB read and certain crash.
Bug report from Royce M <royce@xchglabs.com>
Location: forward.c:713, edns0.c:421
With --add-subnet enabled, process_reply() passes the OPT record
length (~23 bytes) instead of the packet length to check_source().
All internal bounds checks fail, and the function always returns 1.
ECS source validation per RFC 7871 Section 9.2 is completely bypassed.
Bug reported bt Royce M <royce@xchglabs.com>
Location: helper.c:265-270
DHCPv6 CLIDs can be up to 65535 bytes. When --dhcp-script is configured,
the helper hex-encodes raw CLID bytes via sprintf("%.2x") into daemon->packet (5131 bytes).
A 1000-byte CLID writes ~3000 bytes. The helper process retains root privileges.
Note: log6_packet() correctly caps CLID to 100 bytes for logging, but the helper code path was missed.
Bug report from Royce M <royce@xchglabs.com>
This avoids crafted packets which give a value for rdlen _less_
then the space taken up by the fixed data and the signer's name
and engender a negative calculated length for the signature.
Report from Royce M <royce@xchglabs.com>.
Location: dnssec.c:1290-1306, dnssec.c:1450-1463
The bitmap window iteration advances by p[1] instead of p[1]+2
(missing the 2-byte window header). With bitmap_length=0, both rdlen and p are
unchanged, causing an infinite loop and dnsmasq stops responding to all queries.
Reachable before RRSIG validation
(confirmed by the source comment at line 2125), so no valid
DNSSEC signatures are needed.
All buffers capable of holding a domain name should be
at least MAXDNAME*2 + 1 bytes long, where MAXDNAME is the maximum
size of a domain name. The accounts for the trailing zero and the
fact that some characters are escaped in the internal representation
of a domain name in dnsmasq.
The declaration of struct bigname get this wrong, with the effect
that a remote attacker capable of asking DNS queries or answering DNS
queries can cause a large OOB write in the heap.
This was first spotted by Andrew S. Fasano.
In find_soa() extract_name() is called with extrabytes=0 when parsing NS
record names, which means it only validates that the DNS name fits
within the packet but does not check that 10 additional bytes exist for
the type/class/TTL/rdlen fixed fields. Lines 546-549 then
unconditionally read these 10 bytes via GETSHORT/GETLONG macros. An
attacker controlling a DNS zone can craft a NXDOMAIN response where the
NS record name extends to the packet boundary, causing a 10-byte
out-of-bounds read past the valid packet data (CWE-125, CVSS 5.3
Medium). The read stays within the over-allocated packet buffer in
default configurations, limiting crash risk, but accesses data outside
the logical packet boundary. Under certain conditions, the overread may
access stale heap data from prior transactions.
The fix is straightforward: change the extrabytes
argument from 0 to 10, consistent with other call sites in
the same file.
Credit is due to do litli for finding this problem.
Dominik
Dominik
rdevshp
RamSet
Simon Kelley
yubiuser
dependabot[bot]
darkexplosiveqwx
do litli