embeddedartistry / libc

libc targeted for embedded systems usage. Reduced set of functionality (due to embedded nature). Chosen for portability and quick bringup.
MIT License
504 stars 67 forks source link

strlen Address Sanitizer failure #136

Open phillipjohnston opened 4 years ago

phillipjohnston commented 4 years ago
2/2 libc_tests                              FAIL     1.03 s (killed by signal 6 SIGABRT)

Ok:                    1
Expected Fail:         0
Fail:                  1
Unexpected Pass:       0
Skipped:               0
Timeout:               0

The output from the failed tests:

2/2 libc_tests                              FAIL     1.03 s (killed by signal 6 SIGABRT)

--- command ---
18:42:02 CMOCKA_XML_FILE='/Users/pjohnston/src/ea/course-code/src/002-cross-platform-build-system/meson/011-tooling/buildresults/test/%g.xml' /Users/pjohnston/src/ea/course-code/src/002-cross-platform-build-system/meson/011-tooling/buildresults/test/libc_test
--- stderr ---
=================================================================
==30630==ERROR: AddressSanitizer: global-buffer-overflow on address 0x00010d7f4880 at pc 0x00010d7de9fd bp 0x7ffee2457810 sp 0x7ffee2457808
READ of size 8 at 0x00010d7f4880 thread T0
    #0 0x10d7de9fc in strlen strlen.c:95
    #1 0x10d7b2ecd in check_input memmem.c:57
    #2 0x10d853a6e in cmocka_run_one_test_or_fixture (libcmocka.0.dylib:x86_64+0x5a6e)
    #3 0x10d851f57 in _cmocka_run_group_tests (libcmocka.0.dylib:x86_64+0x3f57)
    #4 0x10d7b2a9d in memmem_tests memmem.c:90
    #5 0x10d7b5339 in string_tests string_tests.c:24
    #6 0x10d7a92ff in main main.c:24
    #7 0x7fff6b8433d4 in start (libdyld.dylib:x86_64+0x163d4)

0x00010d7f4881 is located 0 bytes to the right of global variable '<string literal>' defined in '../test/string/memmem.c:57:2' (0x10d7f4880) of size 1
  '<string literal>' is ascii string ''
SUMMARY: AddressSanitizer: global-buffer-overflow strlen.c:95 in strlen
Shadow bytes around the buggy address:
  0x100021afe8c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x100021afe8d0: 00 00 00 00 01 f9 f9 f9 f9 f9 f9 f9 00 00 00 00
  0x100021afe8e0: 01 f9 f9 f9 f9 f9 f9 f9 00 00 00 00 00 00 00 00
  0x100021afe8f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x100021afe900: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x100021afe910:[01]f9 f9 f9 f9 f9 f9 f9 00 00 00 00 00 00 00 00
  0x100021afe920: 00 00 00 00 00 00 00 00 00 00 00 00 01 f9 f9 f9
  0x100021afe930: f9 f9 f9 f9 00 00 00 00 00 00 00 00 00 00 00 00
  0x100021afe940: 00 00 00 00 00 f9 f9 f9 f9 f9 f9 f9 00 00 00 00
  0x100021afe950: 00 00 00 00 01 f9 f9 f9 f9 f9 f9 f9 00 00 00 00
  0x100021afe960: 00 02 f9 f9 f9 f9 f9 f9 00 02 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
  Shadow gap:              cc
==30630==ABORTING
smcpeak commented 2 years ago

The reason for this ASan complaint is strlen reads its data in units of unsigned long, which means it can read more bytes than are in the object containing the string, which has undefined behavior per C11 6.5.6/8. I ran into this with my pointer verifier tool.

The Stack Overflow question Why does glibc's strlen need to be so complicated to run quickly? discusses a similar situation in glibc. My short summary of that discussion is "it's ok because glibc is meant to be used with GCC".

However, my understanding is the EA libc is meant to work with any standard-compliant C compiler, so at least in theory, this strlen could be problematic. But, as with issue #181, I'm just noting it and moving on.

But, I might suggest adding "strlen" to the issue title to distinguish it from other potential ASan findings.