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

Heap buffer overflow reported by Address Sanitizer #280

Merged
merged 2 commits into from
Nov 17, 2022
Merged

Conversation

Isty001
Copy link
Member

@Isty001 Isty001 commented Nov 17, 2022

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
Member

That's interesting! 🤞

Copy link
Member

@jwerle jwerle left a comment

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