[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 1/2] printf: fix heap buffer overflow in printf_builtin
From: |
Martin D Kealey |
Subject: |
Re: [PATCH 1/2] printf: fix heap buffer overflow in printf_builtin |
Date: |
Sat, 31 Aug 2024 00:41:53 +1000 |
Hi Andrei
Ok, I see the problem.
This fault is triggered when the format string has '%(' but is missing the
closing ')' - so the entire remainder of the format string is tentatively
recorded as the time-format substring.
This line:
if (*++fmt != 'T')
should be changed to:
if (n > 0 || *++fmt != 'T')
or perhaps:
if (*fmt == 0 || *++fmt != 'T')
(Personally I would prefer the former, since it would still reject
unbalanced parentheses even if some later code change avoids overrunning
the end-of-string.)
I note that the suggested patch amounts to (a slow version of):
if (*fmt != 0 && *++fmt != 'T')
which avoids the overrun but fails to report the error to the user.
-Martin
On Fri, 30 Aug 2024 at 22:28, Андрей Ковалёв <i.not.student@yandex.ru>
wrote:
> Hi there!
>
> I completely understand your point of view. Although I made a few
> mistakes when writing the patch, I wrote patch for a reason. I was doing
> fuzzing testing in bash4, and at some point during fuzzing, ASAN
> (AddressSanitizer) was launched. This problem also existed in the master
> branch, so I wrote a patch to fix it.
>
> Here is the ASAN trigger on the input data that I attached to this email:
>
> ==2==ERROR: AddressSanitizer: heap-buffer-overflow on address
> 0x5080000009f8 at pc 0x55b1ce740ee0 bp 0x7fff5353bf90 sp 0x7fff5353bf88
>
> READ of size 1 at 0x5080000009f8 thread T0
>
> #0 0x55b1ce740edf in printf_builtin
>
> /artifacts/build-aflplusplus/bash-5.2.26/build-bash/builtins/../../builtins/../../builtins/printf.def:492:7
>
> #1 0x55b1ce464738 in execute_builtin
>
> /artifacts/build-aflplusplus/bash-5.2.26/build-bash/../execute_cmd.c:4974:13
>
> #2 0x55b1ce4631ab in execute_builtin_or_function
>
> /artifacts/build-aflplusplus/bash-5.2.26/build-bash/../execute_cmd.c:5488:14
>
> #3 0x55b1ce43c098 in execute_simple_command
>
> /artifacts/build-aflplusplus/bash-5.2.26/build-bash/../execute_cmd.c:4740:13
>
> #4 0x55b1ce430f33 in execute_command_internal
> /artifacts/build-aflplusplus/bash-5.2.26/build-bash/../execute_cmd.c:866:4
>
> #5 0x55b1ce42ddb0 in execute_command
> /artifacts/build-aflplusplus/bash-5.2.26/build-bash/../execute_cmd.c:413:12
>
> #6 0x55b1ce3ab36a in reader_loop
> /artifacts/build-aflplusplus/bash-5.2.26/build-bash/../eval.c:171:8
>
> #7 0x55b1ce3a07aa in main
> /artifacts/build-aflplusplus/bash-5.2.26/build-bash/../shell.c:833:3
>
> #8 0x7f0e8e7bdc8b (/lib64/libc.so.6+0x27c8b) (BuildId:
> 97aecaf3aeb712a8e66d84b5319d6cca2cf5528e)
>
> #9 0x7f0e8e7bdd44 in __libc_start_main (/lib64/libc.so.6+0x27d44)
> (BuildId: 97aecaf3aeb712a8e66d84b5319d6cca2cf5528e)
>
> #10 0x55b1ce2c6ef0 in _start
> (/artifacts/build-aflplusplus/bash-5.2.26/build-bash/bash+0x21cef0)
> (BuildId: be8de6b123ba7c6e8bc2e7fbc1afe38d8c8a487b)
>
> 0x5080000009f8 is located 0 bytes after 88-byte region
> [0x5080000009a0,0x5080000009f8)
>
> allocated by thread T0 here:
>
> #0 0x55b1ce36112f in malloc
> /usr/src/RPM/BUILD/llvm-project-18/compiler-rt/lib/asan/asan_malloc_linux.cpp:68:3
>
>
>
> #1 0x55b1ce6a82fc in xmalloc
> /artifacts/build-aflplusplus/bash-5.2.26/build-bash/../xmalloc.c:114:10
>
> #2 0x55b1ce5426a7 in dequote_string
> /artifacts/build-aflplusplus/bash-5.2.26/build-bash/../subst.c:4891:24
>
> #3 0x55b1ce5a2cbb in glob_expand_word_list
> /artifacts/build-aflplusplus/bash-5.2.26/build-bash/../subst.c:12390:18
>
> #4 0x55b1ce55057d in expand_word_list_internal
> /artifacts/build-aflplusplus/bash-5.2.26/build-bash/../subst.c:13012:13
>
> #5 0x55b1ce550351 in expand_words
> /artifacts/build-aflplusplus/bash-5.2.26/build-bash/../subst.c:12284:11
>
> #6 0x55b1ce439921 in execute_simple_command
> /artifacts/build-aflplusplus/bash-5.2.26/build-bash/../execute_cmd.c:4509:15
>
>
>
> #7 0x55b1ce430f33 in execute_command_internal
> /artifacts/build-aflplusplus/bash-5.2.26/build-bash/../execute_cmd.c:866:4
>
> #8 0x55b1ce42ddb0 in execute_command
> /artifacts/build-aflplusplus/bash-5.2.26/build-bash/../execute_cmd.c:413:12
>
> #9 0x55b1ce3ab36a in reader_loop
> /artifacts/build-aflplusplus/bash-5.2.26/build-bash/../eval.c:171:8
>
> #10 0x55b1ce3a07aa in main
> /artifacts/build-aflplusplus/bash-5.2.26/build-bash/../shell.c:833:3
>
> #11 0x7f0e8e7bdc8b (/lib64/libc.so.6+0x27c8b) (BuildId:
> 97aecaf3aeb712a8e66d84b5319d6cca2cf5528e)
>
> SUMMARY: AddressSanitizer: heap-buffer-overflow
> /artifacts/build-aflplusplus/bash-5.2.26/build-bash/builtins/../../builtins/../../builtins/printf.def:492:7
>
> in printf_builtin
>
> Shadow bytes around the buggy address:
>
> 0x508000000700: fa fa fa fa fd fd fd fd fd fd fd fd fd fd fd fa
>
> 0x508000000780: fa fa fa fa fd fd fd fd fd fd fd fd fd fd fd fa
>
> 0x508000000800: fa fa fa fa fd fd fd fd fd fd fd fd fd fd fd fa
>
> 0x508000000880: fa fa fa fa fd fd fd fd fd fd fd fd fd fd fd fa
>
> 0x508000000900: fa fa fa fa fd fd fd fd fd fd fd fd fd fd fd fd
>
> =>0x508000000980: fa fa fa fa 00 00 00 00 00 00 00 00 00 00 00[fa]
>
> 0x508000000a00: fa fa fa fa 00 00 00 00 00 00 00 00 00 00 00 05
>
> 0x508000000a80: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
>
> 0x508000000b00: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
>
> 0x508000000b80: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
>
> 0x508000000c00: 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
>
> ==2==ABORTING