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

clif: Don't access any data if input is empty #15947

Merged
merged 1 commit into from Feb 15, 2021

Conversation

nmeum
Copy link
Member

@nmeum nmeum commented Feb 8, 2021

Contribution description

This is relevant as clif_decode_link may invoke clif_get_attr with input_len == 0.

Testing procedure

Example application:

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

#define MAX_EXPECTED_ATTRS (6)
static uint8_t buf[] = { 60, 193, 62, 59, 0, 0, 0, 0, 0, 0, };

int main(void)
{
        char *input = (char *)&buf[0];
	clif_t lookif;
	clif_attr_t attrs[MAX_EXPECTED_ATTRS];

	clif_decode_link(&lookif, attrs, MAX_EXPECTED_ATTRS, input, sizeof(buf));

	return 0;
}

Minimal Makefile:

APPLICATION = clif

BOARD = native

USEMODULE += clif

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

Invoke using:

$ make -C examples/clif all-asan
$ make -C examples/clif term
main(): This is RIOT! (Version: 2021.04-devel-461-g50cf93)
=================================================================
==8410==ERROR: AddressSanitizer: global-buffer-overflow on address 0x566670ea at pc 0x56657452 bp 0x5666c758 sp 0x5666c74c
READ of size 1 at 0x566670ea thread T0
    #0 0x56657451 in clif_get_attr /root/RIOT/sys/clif/clif.c:262
    #1 0x5665704e in clif_decode_link /root/RIOT/sys/clif/clif.c:93
    #2 0x56656b8a in main /root/RIOT/examples/clif/main.c:15
    #3 0x56657b88 in main_trampoline /root/RIOT/core/init.c:58
    #4 0xf769c53a in makecontext (/lib/i386-linux-gnu/libc.so.6+0x4153a)

0x566670ea is located 0 bytes to the right of global variable 'buf' defined in '/root/RIOT/examples/clif/main.c:7:16' (0x566670e0) of size 10
SUMMARY: AddressSanitizer: global-buffer-overflow /root/RIOT/sys/clif/clif.c:262 in clif_get_attr
Shadow bytes around the buggy address:
  0x2acccdc0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x2acccdd0: f9 f9 f9 f9 00 00 00 00 00 00 00 00 00 00 00 00
  0x2acccde0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x2acccdf0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x2accce00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x2accce10: 00 00 00 00 00 00 00 00 00 00 00 00 00[02]f9 f9
  0x2accce20: f9 f9 f9 f9 00 00 00 00 f9 f9 f9 f9 f9 f9 f9 f9
  0x2accce30: 00 00 00 00 00 00 f9 f9 f9 f9 f9 f9 00 00 00 00
  0x2accce40: f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9
  0x2accce50: f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9
  0x2accce60: f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9
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
==8410==ABORTING
make: *** [/root/RIOT/examples/clif/../../Makefile.include:725: term] Error 1
make: Leaving directory '/root/RIOT/examples/clif'

Issues/PRs references

This is conceptually similar but not addressed in #15945.

This is relevant as clif_decode_link may invoke clif_get_attr with
input_len == 0.
@leandrolanzieri leandrolanzieri added Area: sys Area: System CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) labels Feb 8, 2021
@leandrolanzieri leandrolanzieri added this to the Release 2021.04 milestone Feb 8, 2021
@benpicco
Copy link
Contributor

benpicco commented Feb 8, 2021

Since you already wrote some example code, want to also add that to tests/unittests/tests-clif/tests-clif.c?

This case is pretty trivial though, still it had not been spotted so far.

@nmeum
Copy link
Member Author

nmeum commented Feb 10, 2021

Test case added.

@leandrolanzieri leandrolanzieri merged commit 85da504 into RIOT-OS:master Feb 15, 2021
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: sys Area: System CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants