Skip to content

Heap buffer overflow reported by Address Sanitizer#280

Merged
Isty001 merged 2 commits into
masterfrom
basename
Nov 17, 2022
Merged

Heap buffer overflow reported by Address Sanitizer#280
Isty001 merged 2 commits into
masterfrom
basename

Conversation

@Isty001

@Isty001 Isty001 commented Nov 17, 2022

Copy link
Copy Markdown
Member

Probably (:crossed_fingers:) this is root cause of the randomly failing mac tests.

According to man 3 basename These functions may return pointers to statically allocated memory which may be overwritten by subsequent calls., and as it can be seen in the messy logs in #272 I've added there it's true on that environment, since the parallel threads keep overriding the same address. Then path_join allocates N bytes but when it tries to concat the two strings the value of file is changed to something larger thus causing an overflow.

As I can see this is the only place where basename is used concurrently and might cause this error, but let me know if there are other problematic usages.

ASAN output:

1 warning generated.
  (✓) test/install-binary-dependencies.sh
  (✓) test/install-brace-expansion.sh
  (✓) test/install-deps-from-package-json.sh
=================================================================
==4026==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60200005ba60 at pc 0x00010cc7f077 bp 0x70000ae37e00 sp 0x70000ae375a0
WRITE of size 8 at 0x60200005ba60 thread T10
    #0 0x10cc7f076 in wrap_strcat+0x426 (libclang_rt.asan_osx_dynamic.dylib:x86_64+0x44076)
    #1 0x10c5784d7 in path_join path-join.c:54
    #2 0x10c5263a0 in fetch_package_file_work clib-package.c:922
    #3 0x10c526017 in fetch_package_file_thread clib-package.c:[98](https://github.com/clibs/clib/actions/runs/3479569741/jobs/5819251549#step:4:99)9
    #4 0x7ff806e1a4e0 in _pthread_start+0x7c (libsystem_pthread.dylib:x86_64+0x64e0)
    #5 0x7ff806e15f6a in thread_start+0xe (libsystem_pthread.dylib:x86_64+0x1f6a)

0x60200005ba60 is located 0 bytes to the right of 16-byte region [0x60200005ba50,0x60200005ba60)
allocated by thread T10 here:
    #0 0x10cc84f60 in wrap_malloc+0xa0 (libclang_rt.asan_osx_dynamic.dylib:x86_64+0x49f60)
    #1 0x10c5783c3 in path_join path-join.c:31
    #2 0x10c5263a0 in fetch_package_file_work clib-package.c:922
    #3 0x10c526017 in fetch_package_file_thread clib-package.c:989
    #4 0x7ff806e1a4e0 in _pthread_start+0x7c (libsystem_pthread.dylib:x86_64+0x64e0)
    #5 0x7ff806e15f6a in thread_start+0xe (libsystem_pthread.dylib:x86_64+0x1f6a)

Thread T10 created by T0 here:
    #0 0x10cc7ea3c in wrap_pthread_create+0x5c (libclang_rt.asan_osx_dynamic.dylib:x86_64+0x43a3c)
    #1 0x10c524fab in fetch_package_file clib-package.c:[103](https://github.com/clibs/clib/actions/runs/3479569741/jobs/5819251549#step:4:104)2
    #2 0x10c523f3e in clib_package_install clib-package.c:1460
    #3 0x10c52985c in install_package clib-install.c:306
    #4 0x10c528e18 in install_packages clib-install.c:339
    #5 0x10c5288c7 in main clib-install.c:441
    #6 0x[114](https://github.com/clibs/clib/actions/runs/3479569741/jobs/5819251549#step:4:115)bdd52d in start+0x1cd (dyld:x86_64+0x552d)

SUMMARY: AddressSanitizer: heap-buffer-overflow (libclang_rt.asan_osx_dynamic.dylib:x86_64+0x44076) in wrap_strcat+0x426
Shadow bytes around the buggy address:
  0x1c040000b6f0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x1c040000b700: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x1c040000b710: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x1c040000b720: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x1c040000b730: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
=>0x1c040000b740: fa fa fa fa fa fa 04 fa fa fa 00 00[fa]fa fa fa
  0x1c040000b750: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x1c040000b760: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x1c040000b770: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x1c040000b780: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x1c040000b790: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
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
==4026==ABORTING

man 3 basename

These functions may return pointers to statically allocated memory which may be overwritten by subsequent  calls.

And beacuse of this the same address is used in parallel threads
See #272
man 3 basename

These functions may return pointers to statically allocated memory which may be overwritten by subsequent  calls.

And beacuse of this the same address is used in parallel threads
See #272
@Isty001 Isty001 requested review from diasbruno and jwerle November 17, 2022 12:42
@diasbruno

Copy link
Copy Markdown
Member

That's interesting! 🤞

@jwerle jwerle left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice catch!

@Isty001 Isty001 merged commit f12aed5 into master Nov 17, 2022
@Isty001 Isty001 deleted the basename branch November 17, 2022 13:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants