Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gnrc_rpl: missing bounds checks in gnrc_rpl_validation_options #16062

Closed
nmeum opened this issue Feb 22, 2021 · 0 comments · Fixed by #16081
Closed

gnrc_rpl: missing bounds checks in gnrc_rpl_validation_options #16062

nmeum opened this issue Feb 22, 2021 · 0 comments · Fixed by #16081
Assignees
Labels
Area: network Area: Networking Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)

Comments

@nmeum
Copy link
Member

nmeum commented Feb 22, 2021

Description

The gnrc_rpl_validation_options function has a problem very similar to the one described in #16018: It casts packed structs without performing a prior bounds check. For example, consider the handler for GNRC_RPL_OPT_PAD1:

case (GNRC_RPL_OPT_PAD1):
expected_len += 1;
opt = (gnrc_rpl_opt_t *) (((uint8_t *) opt) + 1);
continue;

This is missing a check ala.:

if (len < sizeof(gnrc_rpl_opt_t))
  return false;

Otherwise, reading opt->length (or opt->type on the next iteration) may result in an out-of-bounds read:

expected_len += opt->length + sizeof(gnrc_rpl_opt_t);

Steps to reproduce the issue

Use examples/gnrc_networking, activate gnrc_pktbuf_malloc and set CONFIG_GNRC_RPL_DEFAULT_NETIF to your netif (check with ifconfig in the shell provided by gnrc_networking) mine is 6:

diff --git a/examples/gnrc_networking/Makefile b/examples/gnrc_networking/Makefile
index bf76931305..15132894c1 100644
--- a/examples/gnrc_networking/Makefile
+++ b/examples/gnrc_networking/Makefile
@@ -31,6 +31,9 @@ USEMODULE += netstats_l2
 USEMODULE += netstats_ipv6
 USEMODULE += netstats_rpl
 
+USEMODULE += gnrc_pktbuf_malloc
+CFLAGS += -DCONFIG_GNRC_RPL_DEFAULT_NETIF=6
+
 # Optionally include DNS support. This includes resolution of names at an
 # upstream DNS server and the handling of RDNSS options in Router Advertisements
 # to auto-configure that upstream DNS server.

Compile and run the application using:

$ make -C examples/gnrc_networking all-asan
$ make -C examples/gnrc_networking term

Afterwards run socat as:

# printf 'mwEIQQIMFLsAu4lCkEOUY4EgSlBCrqi7YEKAMwAC' | base64 -d | socat -u STDIN IP6-SENDTO:[<LL address>%tap0]:58

Expected results

The application shouldn't crash.

Actual results

=================================================================
==8073==ERROR: AddressSanitizer: heap-buffer-overflow on address 0xf510050e at pc 0x56656dab bp 0x566bb748 sp 0x566bb73c
READ of size 1 at 0xf510050e thread T0
    #0 0x56656daa in gnrc_rpl_validation_options /root/RIOT/sys/net/gnrc/routing/rpl/gnrc_rpl_validation.c:99
    #1 0x5664c03f in _parse_options /root/RIOT/sys/net/gnrc/routing/rpl/gnrc_rpl_control_messages.c:511
    #2 0x5664efe1 in gnrc_rpl_recv_DIO /root/RIOT/sys/net/gnrc/routing/rpl/gnrc_rpl_control_messages.c:786
    #3 0x566456d4 in _receive /root/RIOT/sys/net/gnrc/routing/rpl/gnrc_rpl.c:185
    #4 0x566463b4 in _event_loop /root/RIOT/sys/net/gnrc/routing/rpl/gnrc_rpl.c:312
    #5 0xf768f53a in makecontext (/lib/i386-linux-gnu/libc.so.6+0x4153a)

0xf510050e is located 0 bytes to the right of 30-byte region [0xf51004f0,0xf510050e)
allocated by thread T0 here:
    #0 0xf7a235d4 in __interceptor_malloc (/lib32/libasan.so.5+0xeb5d4)

SUMMARY: AddressSanitizer: heap-buffer-overflow /root/RIOT/sys/net/gnrc/routing/rpl/gnrc_rpl_validation.c:99 in gnrc_rpl_validation_options
Shadow bytes around the buggy address:
  0x3ea20050: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x3ea20060: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x3ea20070: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x3ea20080: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x3ea20090: fa fa fa fa fa fa fa fa fa fa fa fa fa fa 00 00
=>0x3ea200a0: 00[06]fa fa 00 00 04 fa fa fa 00 00 04 fa fa fa
  0x3ea200b0: 00 00 04 fa fa fa fd fd fd fa fa fa 00 00 04 fa
  0x3ea200c0: fa fa fd fd fd fa fa fa fd fd fd fa fa fa fd fd
  0x3ea200d0: fd fa fa fa fd fd fd fa fa fa fd fd fd fa fa fa
  0x3ea200e0: fd fd fd fa fa fa fd fd fd fd fa fa fd fd fd fa
  0x3ea200f0: fa fa fd fd fd fa fa fa fd fd fd fa fa fa fd fd
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
==8073==ABORTING
make: *** [/root/RIOT/examples/gnrc_networking/../../Makefile.include:729: term] Error 1
make: Leaving directory '/root/RIOT/examples/gnrc_networking'

CC: @cgundogan

@cgundogan cgundogan self-assigned this Feb 22, 2021
@cgundogan cgundogan added Area: network Area: Networking Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) labels Feb 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: network Area: Networking Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants