Fix for #15353 is incomplete

[nmeum]

Description

Just managed to get around to testing the fix for #15353 implemented in #15359. Unfortunately, it seems to me that the proposed fix is incomplete. I was sadly busy with other things and only managed to get around to testing it today. Please let me know if you can also reproduce the issue with this new provided test case.

Steps to reproduce the issue

Application code:

#include <stdio.h>
#include <stdlib.h>
#include <stdint.h>
#include <panic.h>

#include <netinet/in.h>
#include <net/uhcp.h>

static uint8_t buf[] = { 85, 72, 67, 80, 1, 252, 0, 0, 0, 0, 0, 0, 0, 0,
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, };

void uhcp_handle_prefix(uint8_t *prefix, uint8_t prefix_len, uint16_t lifetime,
                        uint8_t *src, uhcp_iface_t iface) {
    (void)prefix;
    (void)prefix_len;
    (void)lifetime;
    (void)src;
    (void)iface;
    return;
}

int main(void)
{
    uint8_t addr_buf[16];

    memset(addr_buf, '\0', 16);
    uhcp_handle_udp(buf, sizeof(buf), &addr_buf[0], 23, 0);

    exit(EXIT_SUCCESS);
    return 0;
}

Makefile:

APPLICATION = uhcp

BOARD = native

# Required modules
USEMODULE += gnrc_ipv6
USEMODULE += gnrc_sock_udp
USEMODULE += uhcpc

# Enable DHCP client
CFLAGS += -DUHCP_CLIENT

RIOTBASE ?= $(CURDIR)/../..
include $(RIOTBASE)/Makefile.include

Compile and run with:

$ make BOARD=native -C examples/uhcpc/ all-asan
$ make BOARD=native -C examples/uhcpc/ term

Expected results

The application should not crash.

Actual results

==30861==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x5660aa00 at pc 0xf79aa437 bp 0x5660a998 sp 0x5660a570
WRITE of size 32 at 0x5660aa00 thread T0
    #0 0xf79aa436  (/lib32/libasan.so.5+0x37436)
    #1 0x565efc9b in uhcp_handle_push /root/RIOT/sys/net/application_layer/uhcp/uhcp.c:106
    #2 0x565efa1f in uhcp_handle_udp /root/RIOT/sys/net/application_layer/uhcp/uhcp.c:63
    #3 0x565c5c24 in main /root/RIOT/examples/uhcpc/main.c:28
    #4 0x565c5f91 in main_trampoline /root/RIOT/core/init.c:57
    #5 0xf76ca53a in makecontext (/lib/i386-linux-gnu/libc.so.6+0x4153a)

0x5660aa00 is located 11200 bytes inside of global variable 'main_stack' defined in '/root/RIOT/core/init.c:62:13' (0x56607e40) of size 12288
SUMMARY: AddressSanitizer: stack-buffer-overflow (/lib32/libasan.so.5+0x37436) 
Shadow bytes around the buggy address:
  0x2acc14f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x2acc1500: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x2acc1510: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x2acc1520: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x2acc1530: 00 00 00 00 00 00 00 00 00 00 f1 f1 f1 f1 00 00
=>0x2acc1540:[f2]f2 f2 f2 f2 f2 00 00 00 00 00 06 f2 f2 f2 f2
  0x2acc1550: f2 f2 00 00 00 00 00 06 f2 f2 f3 f3 f3 f3 00 00
  0x2acc1560: 00 00 00 00 00 00 00 00 f1 f1 f1 f1 00 00 00 00
  0x2acc1570: 00 06 f2 f2 f3 f3 f3 f3 00 00 00 00 00 00 00 00
  0x2acc1580: f1 f1 f1 f1 00 00 f2 f2 f3 f3 f3 f3 00 00 00 00
  0x2acc1590: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==30861==ABORTING
make: *** [/root/RIOT/examples/uhcpc/../../Makefile.include:726: term] Error 1

Versions

I don't think this is needed to reproduce the issue, can provide if needed. RIOT head f74cb05.

[miri64]

True. One should also check if the prefix length does not go over the maximum allowed value. I have a fix, I just need to test if your proposed test is now working with the fix.

[miri64]

See #15603