fuzz: FuzzedFileProvider::write should not return negative value #24449

Crypt-iQ commented at 10:37 PM on February 27, 2022: contributor

Doing so can lead to a glibc crash (from 2005 but I think it's relevant https://sourceware.org/bugzilla/show_bug.cgi?id=2074). Also the manpage for fopencookie warns against this: https://man7.org/linux/man-pages/man3/fopencookie.3.html. This would invalidate the autofile seeds (and maybe others?) in qa-assets.

On another note, I noticed that FuzzedFileProvider::seek has some confusing behavior with SEEK_END. It seems to me that if these handlers are supposed to mimic the real functions, that SEEK_END would use the offset from the end of the stream, rather than changing the offset with a random value between 0 and 4096. I could also open a PR to fix SEEK_END, but it would invalidate the seeds.

fuzz: FuzzedFileProvider::write should not return negative value

Doing so can lead to a glibc crash. Also the manpage for fopencookie
warns against this: https://man7.org/linux/man-pages/man3/fopencookie.3.html

fc471814dc

DrahtBot added the label Tests on Feb 28, 2022

fanquake requested review from MarcoFalke on Feb 28, 2022

MarcoFalke commented at 7:34 AM on February 28, 2022: member

cr ACK fc471814dc34abb4d5479803ebb1033b572eda43

MarcoFalke merged this on Feb 28, 2022

MarcoFalke closed this on Feb 28, 2022

MarcoFalke commented at 8:05 AM on February 28, 2022: member

Just to confirm, there are no steps to reproduce?

Crypt-iQ commented at 2:11 PM on February 28, 2022: contributor

No steps to reproduce. My earlier comment was wrong, this shouldn't change any of the seeds since triggering an overflow of a int64_t at steps with maximum size of 4096 is infeasible.

Crypt-iQ deleted the branch on Feb 28, 2022

MarcoFalke commented at 2:23 PM on February 28, 2022: member

On another note, I noticed that FuzzedFileProvider::seek has some confusing behavior with SEEK_END. It seems to me that if these handlers are supposed to mimic the real functions, that SEEK_END would use the offset from the end of the stream, rather than changing the offset with a random value between 0 and 4096. I could also open a PR to fix SEEK_END, but it would invalidate the seeds.

I wonder if this is related to https://bugs.chromium.org/p/oss-fuzz/issues/list?q=FuzzedFileProvider&can=2

Crypt-iQ commented at 3:31 PM on February 28, 2022: contributor

That is what I was trying to figure out, but didn't finish setting up oss-fuzz. I think FuzzedFileProvider would have to track the end of the filestream

sidhujag referenced this in commit ad7820abcd on Mar 1, 2022

Crypt-iQ commented at 1:10 PM on March 22, 2022: contributor

Not sure what the right place to post this is, but posting here. It seems that the offset value under ASAN+i386 is converted to a 32-bit pointer that's passed into FuzzedFileProvider::seek and that is the reason for the crash. I modified the autofile harness to only call (void)fseek(file, 12, SEEK_CUR); and exit. Here file is one created from fopencookie and the fuzzed file provider. This was the result:

AddressSanitizer:DEADLYSIGNAL
=================================================================
==12==ERROR: AddressSanitizer: SEGV on unknown address 0x0000000c (pc 0x583fa6d4 bp 0xff8722e8 sp 0xff872240 T0)
==12==The signal is caused by a READ memory access.
==12==Hint: address points to the zero page.
SCARINESS: 10 (null-deref)
    [#0](/bitcoin-bitcoin/0/) 0x583fa6d4 in FuzzedFileProvider::seek(void*, long long*, int) /src/bitcoin-core/src/test/fuzz/util.cpp:621:39
    [#1](/bitcoin-bitcoin/1/) 0x56be7d99 in wrapped_seek(void*, unsigned long long*, int) /src/llvm-project/compiler-rt/lib/asan/../sanitizer_common/sanitizer_common_interceptors.inc:6454:22
    [#2](/bitcoin-bitcoin/2/) 0xf7cde492  (/lib32/libc.so.6+0x141492)
    [#3](/bitcoin-bitcoin/3/) 0xf7c14999 in _IO_file_seekoff (/lib32/libc.so.6+0x77999)
    [#4](/bitcoin-bitcoin/4/) 0xf7c10bc8 in fseek (/lib32/libc.so.6+0x73bc8)
    [#5](/bitcoin-bitcoin/5/) 0x56cc20ac in autofile_fuzz_target(Span<unsigned char const>) /src/bitcoin-core/src/test/fuzz/autofile.cpp:25:11
    [#6](/bitcoin-bitcoin/6/) 0x56c716f6 in __invoke<void (*&)(Span<const unsigned char>), Span<const unsigned char> > /usr/local/bin/../include/c++/v1/type_traits:3663:23
    [#7](/bitcoin-bitcoin/7/) 0x56c716f6 in __call<void (*&)(Span<const unsigned char>), Span<const unsigned char> > /usr/local/bin/../include/c++/v1/__functional/invoke.h:61:9
    [#8](/bitcoin-bitcoin/8/) 0x56c716f6 in operator() /usr/local/bin/../include/c++/v1/__functional/function.h:172:16
    [#9](/bitcoin-bitcoin/9/) 0x56c716f6 in std::__1::__function::__func<void (*)(Span<unsigned char const>), std::__1::allocator<void (*)(Span<unsigned char const>)>, void (Span<unsigned char const>)>::operator()(Span<unsigned char const>&&) /usr/local/bin/../include/c++/v1/__functional/function.h:346:12
    [#10](/bitcoin-bitcoin/10/) 0x583dca19 in operator() /usr/local/bin/../include/c++/v1/__functional/function.h:499:16
    [#11](/bitcoin-bitcoin/11/) 0x583dca19 in operator() /usr/local/bin/../include/c++/v1/__functional/function.h:1176:12
    [#12](/bitcoin-bitcoin/12/) 0x583dca19 in LLVMFuzzerTestOneInput /src/bitcoin-core/src/test/fuzz/fuzz.cpp:119:5
    [#13](/bitcoin-bitcoin/13/) 0x56b3fc6d in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned int) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:611:15
    [#14](/bitcoin-bitcoin/14/) 0x56b40f66 in fuzzer::Fuzzer::ReadAndExecuteSeedCorpora(std::__Fuzzer::vector<fuzzer::SizedFile, std::__Fuzzer::allocator<fuzzer::SizedFile> >&) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:804:3
    [#15](/bitcoin-bitcoin/15/) 0x56b4140e in fuzzer::Fuzzer::Loop(std::__Fuzzer::vector<fuzzer::SizedFile, std::__Fuzzer::allocator<fuzzer::SizedFile> >&) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:857:3
    [#16](/bitcoin-bitcoin/16/) 0x56b30c6d in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned int)) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:912:6
    [#17](/bitcoin-bitcoin/17/) 0x56b59a67 in main /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10
    [#18](/bitcoin-bitcoin/18/) 0xf7bb7ee4 in __libc_start_main (/lib32/libc.so.6+0x1aee4)
    [#19](/bitcoin-bitcoin/19/) 0x56b09114 in _start (/out/autofile+0x59b114)

DEDUP_TOKEN: FuzzedFileProvider::seek(void*, long long*, int)--wrapped_seek(void*, unsigned long long*, int)--
AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV /src/bitcoin-core/src/test/fuzz/util.cpp:621:39 in FuzzedFileProvider::seek(void*, long long*, int)
==12==ABORTING
MS: 0 ; base unit: 0000000000000000000000000000000000000000

Since 0xc = 12, this leads me to believe my above statement. This also makes me think this is the reason why the crashes I've observed without modifying autofile were in the high 0xffff.... range (small negative number converted to 32-bit pointer). This seems to either be:

a problem with ASAN's fopencookie wrapper which calls wrapped_seek
a problem with bitcoind's build for ASAN + i386

MarcoFalke commented at 2:22 PM on March 22, 2022: member

Interesting. I wonder where this happens, as the reproducer is 8KB, so it seems to only happen after some "state" is accumulated.

Crypt-iQ commented at 5:00 PM on March 22, 2022: contributor

In the stripped-down autofile test I created, it only happened with ASAN (non-ASAN builds correctly used a heap pointer to the value) and it occurred on the very first run since FuzzedFileProvider::seek was immediately called via fseek. When running the regular autofile harness with ASAN, it would happen on the first call to FuzzedFileProvider::seek, which could take hundreds of thousands of iterations. When I reproduced it, it would only happen in r+ mode after fclose which would flush the pending writes, call FuzzedFileProvider::seek, then crash. Granted, I did modify the regular autofile harness a little bit, so the crash I received may not be the only callpath to FuzzedFileProvider::seek.

MarcoFalke commented at 3:02 PM on March 23, 2022: member

I finally figured out a way to reproduce this locally on a fresh install of Ubuntu Focal. (Ubuntu Jammy doesn't work due to #24289)

export V=12 && dpkg --add-architecture i386 && export DEBIAN_FRONTEND=noninteractive && apt update && apt install curl wget htop git vim ccache -y && git clone https://github.com/bitcoin/bitcoin.git ./bitcoin-core && cd bitcoin-core && apt install libc++abi-$V-dev:i386 libc++-$V-dev:i386 clang-$V:i386 llvm-$V:i386        make automake cmake curl libtool  bsdmainutils pkg-config patch bison        -y  && ( cd depends && make CC="clang-$V -m32 -O1 -fno-omit-frame-pointer -gline-tables-only -fsanitize=address -fsanitize-address-use-after-scope -fsanitize=fuzzer-no-link" CXX="clang++-$V -m32 -g -O1 -fno-omit-frame-pointer -gline-tables-only -fsanitize=address -fsanitize-address-use-after-scope -fsanitize=fuzzer-no-link -stdlib=libc++" DEBUG=1 NO_QT=1 NO_WALLET=1 NO_ZMQ=1 NO_UPNP=1 NO_NATPMP=1 -j $(nproc) ) && ./autogen.sh && CONFIG_SITE="$PWD/depends/x86_64-pc-linux-gnu/share/config.site" ./configure CC="clang-$V -m32 -O1 -fno-omit-frame-pointer -gline-tables-only" CXX="clang++-$V -m32 -g -O1 -fno-omit-frame-pointer -gline-tables-only -stdlib=libc++" --with-sanitizers=fuzzer,address --enable-fuzz --with-seccomp=no --enable-fuzz  && make -j $(nproc)


wget --quiet https://oss-fuzz.com/download?testcase_id=6045002537500672


FUZZ=autofile ./src/test/fuzz/fuzz ./download\?testcase_id\=6045002537500672

INFO: Running with entropic power schedule (0xFF, 100).
INFO: Seed: 29997060
INFO: Loaded 1 modules   (191707 inline 8-bit counters): 191707 [0x58e7c440, 0x58eab11b), 
INFO: Loaded 1 PC tables (191707 PCs): 191707 [0x58eab11c,0x590217f4), 
./src/test/fuzz/fuzz: Running 1 inputs 1 time(s) each.
Running: ./download?testcase_id=6045002537500672
AddressSanitizer:DEADLYSIGNAL
=================================================================
==20952==ERROR: AddressSanitizer: SEGV on unknown address 0xffffe583 (pc 0x582f6a6b bp 0xfff4ee28 sp 0xfff4ee00 T0)
==20952==The signal is caused by a READ memory access.
    [#0](/bitcoin-bitcoin/0/) 0x582f6a6b in FuzzedFileProvider::seek(void*, long long*, int) src/test/fuzz/util.cpp:584:13
    [#1](/bitcoin-bitcoin/1/) 0x56e090b9 in wrapped_seek(void*, unsigned long long*, int) (/bitcoin-core/src/test/fuzz/fuzz+0x7c80b9)
    [#2](/bitcoin-bitcoin/2/) 0xf7c76d22  (/lib/i386-linux-gnu/libc.so.6+0x143d22)
    [#3](/bitcoin-bitcoin/3/) 0xf7baaa90  (/lib/i386-linux-gnu/libc.so.6+0x77a90)
    [#4](/bitcoin-bitcoin/4/) 0xf7bacade in _IO_do_write (/lib/i386-linux-gnu/libc.so.6+0x79ade)
    [#5](/bitcoin-bitcoin/5/) 0xf7bacf4a in _IO_file_overflow (/lib/i386-linux-gnu/libc.so.6+0x79f4a)
    [#6](/bitcoin-bitcoin/6/) 0xf7bada19 in _IO_switch_to_get_mode (/lib/i386-linux-gnu/libc.so.6+0x7aa19)
    [#7](/bitcoin-bitcoin/7/) 0xf7badc64 in __underflow (/lib/i386-linux-gnu/libc.so.6+0x7ac64)
    [#8](/bitcoin-bitcoin/8/) 0xf7bae248 in _IO_default_xsgetn (/lib/i386-linux-gnu/libc.so.6+0x7b248)
    [#9](/bitcoin-bitcoin/9/) 0xf7b9f546 in fread (/lib/i386-linux-gnu/libc.so.6+0x6c546)
    [#10](/bitcoin-bitcoin/10/) 0x56dd06be in fread (/bitcoin-core/src/test/fuzz/fuzz+0x78f6be)
    [#11](/bitcoin-bitcoin/11/) 0x56ef5356 in CAutoFile::read(Span<std::byte>) src/./streams.h:532:13
    [#12](/bitcoin-bitcoin/12/) 0x56ef9824 in unsigned int ser_readdata32<CAutoFile>(CAutoFile&) src/./serialize.h:103:7
    [#13](/bitcoin-bitcoin/13/) 0x56ef963f in void Unserialize<CAutoFile>(CAutoFile&, int&) src/./serialize.h:214:81
    [#14](/bitcoin-bitcoin/14/) 0x56ef9453 in CAutoFile& CAutoFile::operator>><int&>(int&) src/./streams.h:575:9
    [#15](/bitcoin-bitcoin/15/) 0x56ef7512 in void ReadFromStream<CAutoFile>(FuzzedDataProvider&, CAutoFile&)::'lambda4'()::operator()() const src/./test/fuzz/util.h:422:17
    [#16](/bitcoin-bitcoin/16/) 0x56ef6b70 in unsigned int CallOneOf<void ReadFromStream<CAutoFile>(FuzzedDataProvider&, CAutoFile&)::'lambda'(), void ReadFromStream<CAutoFile>(FuzzedDataProvider&, CAutoFile&)::'lambda0'(), void ReadFromStream<CAutoFile>(FuzzedDataProvider&, CAutoFile&)::'lambda1'(), void ReadFromStream<CAutoFile>(FuzzedDataProvider&, CAutoFile&)::'lambda2'(), void ReadFromStream<CAutoFile>(FuzzedDataProvider&, CAutoFile&)::'lambda3'(), void ReadFromStream<CAutoFile>(FuzzedDataProvider&, CAutoFile&)::'lambda4'(), void ReadFromStream<CAutoFile>(FuzzedDataProvider&, CAutoFile&)::'lambda5'(), void ReadFromStream<CAutoFile>(FuzzedDataProvider&, CAutoFile&)::'lambda6'(), void ReadFromStream<CAutoFile>(FuzzedDataProvider&, CAutoFile&)::'lambda7'(), void ReadFromStream<CAutoFile>(FuzzedDataProvider&, CAutoFile&)::'lambda8'(), void ReadFromStream<CAutoFile>(FuzzedDataProvider&, CAutoFile&)::'lambda9'()>(FuzzedDataProvider&, CAutoFile...) src/./test/fuzz/util.h:89:27
    [#17](/bitcoin-bitcoin/17/) 0x56ef6580 in void ReadFromStream<CAutoFile>(FuzzedDataProvider&, CAutoFile&) src/./test/fuzz/util.h:415:13
    [#18](/bitcoin-bitcoin/18/) 0x56ef50f5 in autofile_fuzz_target(Span<unsigned char const>)::$_4::operator()() const src/test/fuzz/autofile.cpp:49:17
    [#19](/bitcoin-bitcoin/19/) 0x56ef4167 in unsigned int CallOneOf<autofile_fuzz_target(Span<unsigned char const>)::$_0, autofile_fuzz_target(Span<unsigned char const>)::$_1, autofile_fuzz_target(Span<unsigned char const>)::$_2, autofile_fuzz_target(Span<unsigned char const>)::$_3, autofile_fuzz_target(Span<unsigned char const>)::$_4, autofile_fuzz_target(Span<unsigned char const>)::$_5>(FuzzedDataProvider&, autofile_fuzz_target(Span<unsigned char const>)::$_0, autofile_fuzz_target(Span<unsigned char const>)::$_1, autofile_fuzz_target(Span<unsigned char const>)::$_2, autofile_fuzz_target(Span<unsigned char const>)::$_3, autofile_fuzz_target(Span<unsigned char const>)::$_4, autofile_fuzz_target(Span<unsigned char const>)::$_5) src/./test/fuzz/util.h:89:27
    [#20](/bitcoin-bitcoin/20/) 0x56ef3999 in autofile_fuzz_target(Span<unsigned char const>) src/test/fuzz/autofile.cpp:23:9
    [#21](/bitcoin-bitcoin/21/) 0x56e72e5e in decltype(std::__1::forward<void (*&)(Span<unsigned char const>)>(fp)(std::__1::forward<Span<unsigned char const> >(fp0))) std::__1::__invoke<void (*&)(Span<unsigned char const>), Span<unsigned char const> >(void (*&)(Span<unsigned char const>), Span<unsigned char const>&&) /usr/lib/llvm-12/bin/../include/c++/v1/type_traits:3694:1
    [#22](/bitcoin-bitcoin/22/) 0x56e72c3a in void std::__1::__invoke_void_return_wrapper<void, true>::__call<void (*&)(Span<unsigned char const>), Span<unsigned char const> >(void (*&)(Span<unsigned char const>), Span<unsigned char const>&&) /usr/lib/llvm-12/bin/../include/c++/v1/__functional_base:348:9
    [#23](/bitcoin-bitcoin/23/) 0x56e72b7a in std::__1::__function::__alloc_func<void (*)(Span<unsigned char const>), std::__1::allocator<void (*)(Span<unsigned char const>)>, void (Span<unsigned char const>)>::operator()(Span<unsigned char const>&&) /usr/lib/llvm-12/bin/../include/c++/v1/functional:1558:16
    [#24](/bitcoin-bitcoin/24/) 0x56e6f343 in std::__1::__function::__func<void (*)(Span<unsigned char const>), std::__1::allocator<void (*)(Span<unsigned char const>)>, void (Span<unsigned char const>)>::operator()(Span<unsigned char const>&&) /usr/lib/llvm-12/bin/../include/c++/v1/functional:1732:12
    [#25](/bitcoin-bitcoin/25/) 0x582ecd7b in std::__1::__function::__value_func<void (Span<unsigned char const>)>::operator()(Span<unsigned char const>&&) const /usr/lib/llvm-12/bin/../include/c++/v1/functional:1885:16
    [#26](/bitcoin-bitcoin/26/) 0x582dfe6d in std::__1::function<void (Span<unsigned char const>)>::operator()(Span<unsigned char const>) const /usr/lib/llvm-12/bin/../include/c++/v1/functional:2560:12
    [#27](/bitcoin-bitcoin/27/) 0x582dfb96 in LLVMFuzzerTestOneInput src/test/fuzz/fuzz.cpp:154:5
    [#28](/bitcoin-bitcoin/28/) 0x56d69f46 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned int) (/bitcoin-core/src/test/fuzz/fuzz+0x728f46)
    [#29](/bitcoin-bitcoin/29/) 0x56d5692e in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned int) (/bitcoin-core/src/test/fuzz/fuzz+0x71592e)
    [#30](/bitcoin-bitcoin/30/) 0x56d5bf74 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned int)) (/bitcoin-core/src/test/fuzz/fuzz+0x71af74)
    [#31](/bitcoin-bitcoin/31/) 0x56d810c7 in main (/bitcoin-core/src/test/fuzz/fuzz+0x7400c7)
    [#32](/bitcoin-bitcoin/32/) 0xf7b4dee4 in __libc_start_main (/lib/i386-linux-gnu/libc.so.6+0x1aee4)
    [#33](/bitcoin-bitcoin/33/) 0x56d33204 in _start (/bitcoin-core/src/test/fuzz/fuzz+0x6f2204)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV src/test/fuzz/util.cpp:584:13 in FuzzedFileProvider::seek(void*, long long*, int)
==20952==ABORTING

MarcoFalke commented at 4:05 PM on March 23, 2022: member

I think this happens when reading a few bytes from an empty file, then writing to it, then closing it?

MarcoFalke commented at 7:46 PM on March 24, 2022: member

Oh, actually the fuzz target doesn't have the concept of empty file when it is opened? I'll try to transform the fuzz input into a unit test ...

Crypt-iQ commented at 8:06 PM on March 24, 2022: contributor

I think this happens when reading a few bytes from an empty file, then writing to it, then closing it?

In the stacktrace you posted above, I only see an fread and then an fwrite (I couldn't really understand the libc source code here, but I think it switches to write mode in certain conditions) rather than an fclose triggering it, so I think there are multiple ways to hit the seek call. I'm going to try making a simple test case outside of bitcoind that just uses fopencookie and asan on i386 to see what happens.

Crypt-iQ commented at 1:08 AM on March 25, 2022: contributor

I believe this is an ASAN issue with fopencookie. I get the following stacktrace when running ASAN on a minimal test case:

AddressSanitizer:DEADLYSIGNAL
=================================================================
==1323==ERROR: AddressSanitizer: SEGV on unknown address 0x0000271f (pc 0x08110fa6 bp 0xffb33f88 sp 0xffb33f50 T0)
==1323==The signal is caused by a READ memory access.
    [#0](/bitcoin-bitcoin/0/) 0x8110fa6 in cookie::cookie_seek(void*, long long*, int) /build/main.cpp:52:22
    [#1](/bitcoin-bitcoin/1/) 0x80b03d9 in wrapped_seek(void*, unsigned long long*, int) (/build/main+0x80b03d9)
    [#2](/bitcoin-bitcoin/2/) 0xf7c1ed22  (/lib/i386-linux-gnu/libc.so.6+0x143d22)
    [#3](/bitcoin-bitcoin/3/) 0xf7b534b9 in _IO_file_seekoff (/lib/i386-linux-gnu/libc.so.6+0x784b9)
    [#4](/bitcoin-bitcoin/4/) 0xf7b4f6c8 in fseek (/lib/i386-linux-gnu/libc.so.6+0x746c8)
    [#5](/bitcoin-bitcoin/5/) 0x811113e in main /build/main.cpp:65:15
    [#6](/bitcoin-bitcoin/6/) 0xf7af5ee4 in __libc_start_main (/lib/i386-linux-gnu/libc.so.6+0x1aee4)
    [#7](/bitcoin-bitcoin/7/) 0x8060355 in _start (/build/main+0x8060355)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV /build/main.cpp:52:22 in cookie::cookie_seek(void*, long long*, int)
==1323==ABORTING

Code is here: https://github.com/Crypt-iQ/fopencookie I ran it with: docker run -it $(docker build -q .) and removing -fsanitize=address makes it pass. The pointer address is the number I used in fseek.

DrahtBot locked this on Mar 25, 2023