>Synopsis: no termination on buffer
>Category: library
>Environment:
System : OpenBSD 7.3
Details : OpenBSD 7.3 (GENERIC.MP) #2080: Sat Mar 25 14:20:25 MDT
2023
dera...@arm64.openbsd.org:/usr/src/sys/arch/arm64/compile/GENERIC.MP
Architecture: OpenBSD.arm64
Machine : arm64
>Description:
This is all just theory as I'm code reading. Let's start in
setup_query() in /usr/src/lib/libc/asr/res_send_async.c ,...
fqdn and dname are size MAXDNAME, pretend MAXDNAME is 50 for simplicity
this means that the maximum fqdn name can be 49 characters long, and
[1] this is checked in _asr_make_fqdn(), the 50th character is \0.
the next function is _asr_dname_from_fqdn() and it does not check for
anything but -1, though perhaps it should check for more? Let's
check this function out, in /usr/src/lib/libc/asr/asr_utils.c
ssize_t
_asr_dname_from_fqdn(const char *str, char *dst, size_t max)
{
XXX str is the fqdn so 49 characters long, dst is dname 50 characters sized.
max is also at 50.
res = 0;
/* special case: the root domain */
if (str[0] == '.') {
XXX we're not a root domain so we skip this check, now the room gets hot
for (; *str; str = d + 1) {
d = strchr(str, '.');
if (d == NULL || d == str)
return (-1);
XXX the 49th character in str is '.' coincidentally.
l = (d - str);
XXX here l == 49
if (dname_check_label(str, l) == -1)
return (-1);
XXX this just checks if we're beyond 63 so it doesn't apply.
res += l + 1;
XXX res is now 49 + 1 == 50
if (dst) {
*dst++ = l;
XXX dst is incremented by 1 byte
max -= 1;
XXX max is now 49
n = (l > max) ? max : l;
XXX l is not larger than max so false, n equals 49 now
memmove(dst, str, n);
XXX dst is now filled with 49 bytes from str
max -= n;
XXX max is reduced by 49 so it is 0
if (max == 0)
dst = NULL;
XXX dst is now NULL
else
XXX else does not apply here, we go through the loop again *str is at 49(.)
for (; *str; str = d + 1) {
XXX at this point the loop ends becuase d is at 49th byte + 1 is the 50th
XXX byte and that is \0, see [1]
if (dst)
XXX dst is NULL so this does not enter the if condition, which is too bad
*dst++ = '\0';
XXX because that would have terminated the dst, even if it was an off by
XXX one.
return (res + 1);
XXX we return res + 1, which is irrelevant because it only gets checked for
-1
So what do we have here?
dname is not terminated! dname was from the stack and not guaranteed
to be zero'ed or is it? Going / returning to:
static int
setup_query(struct asr_query *as, const char *name, const char *dom,
we eventually do strdup(dname), which surely does a strlen() of dname
the dname buffer is overwalked, into somewhere until the \0.
we could have a crash or it could be exploited further down perhaps.
>How-To-Repeat:
Code reading for something, for which I may put up a mail soon.
>Fix:
one thing that can be done is guaranteeing that the buffer of
dname is zero filled with a memset(, 0, sizeof(dname)) at
beginning of setup_query() This would avert a lot of pain.
Or you could fix _asr_dname_from_fqdn(), which I won't do.
dmesg:
OpenBSD 7.3 (GENERIC.MP) #2080: Sat Mar 25 14:20:25 MDT 2023
dera...@arm64.openbsd.org:/usr/src/sys/arch/arm64/compile/GENERIC.MP
real mem = 8432840704 (8042MB)
avail mem = 8139239424 (7762MB)
random: good seed from bootblocks
mainbus0 at root: ACPI
psci0 at mainbus0: PSCI 1.1, SMCCC 1.2
cpu0 at mainbus0 mpidr 0: ARM Cortex-A72 r0p3
cpu0: 48KB 64b/line 3-way L1 PIPT I-cache, 32KB 64b/line 2-way L1 D-cache
cpu0: 1024KB 64b/line 16-way L2 cache
cpu0: CRC32,ASID16
cpu1 at mainbus0 mpidr 1: ARM Cortex-A72 r0p3
cpu1: 48KB 64b/line 3-way L1 PIPT I-cache, 32KB 64b/line 2-way L1 D-cache
cpu1: 1024KB 64b/line 16-way L2 cache
cpu1: CRC32,ASID16
cpu2 at mainbus0 mpidr 2: ARM Cortex-A72 r0p3
cpu2: 48KB 64b/line 3-way L1 PIPT I-cache, 32KB 64b/line 2-way L1 D-cache
cpu2: 1024KB 64b/line 16-way L2 cache
cpu2: CRC32,ASID16
cpu3 at mainbus0 mpidr 3: ARM Cortex-A72 r0p3
cpu3: 48KB 64b/line 3-way L1 PIPT I-cache, 32KB 64b/line 2-way L1 D-cache
cpu3: 1024KB 64b/line 16-way L2 cache
cpu3: CRC32,ASID16
efi0 at mainbus0: UEFI 2.7
efi0: https://github.com/pftf/RPi4 rev 0x1
smbios0 at efi0: SMBIOS 3.3.0
smbios0: vendor https://github.com/pftf/RPi4 version "UEFI
