oss-fuzz/11360: clear out s->prev buffer to avoid undefined behavior

[sebpop]

this patch fixes a use of uninitialized value discovered by one of the fuzzers
of the oss-fuzz project:
https://github.com/google/oss-fuzz/blob/master/projects/zlib/example_dict_fuzzer.c
clang's memory sanitizer fails with:

==1==WARNING: MemorySanitizer: use-of-uninitialized-value
#0 0x5930dd in slide_hash zlib/deflate.c:222:20
#1 0x589461 in fill_window zlib/deflate.c:1558:13
#2 0x59828f in deflate_rle zlib/deflate.c:2119:13
#3 0x590614 in deflate zlib/deflate.c:1045:41
#4 0x4a2d56 in test_dict_deflate /src/example_dict_fuzzer.c:79:11
#5 0x4a3e9b in LLVMFuzzerTestOneInput /src/example_dict_fuzzer.c:156:3
#6 0x4ed04b in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /src/libfuzzer/FuzzerLoop.cpp:571:15
#7 0x4a4ff6 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) /src/libfuzzer/FuzzerDriver.cpp:280:6
#8 0x4b5e1a in fuzzer::FuzzerDriver(int*, char***, int ()(unsigned char const, unsigned long)) /src/libfuzzer/FuzzerDriver.cpp:713:9
#9 0x4a4121 in main /src/libfuzzer/FuzzerMain.cpp:20:10
#10 0x7f3d7a13b82f in __libc_start_main /build/glibc-Cl5G7W/glibc-2.23/csu/libc-start.c:291
#11 0x41ed08 in start
Uninitialized value was created by a heap allocation
#0 0x45fa10 in malloc /src/llvm/projects/compiler-rt/lib/msan/msan_interceptors.cc:911
#1 0x586920 in deflateInit2 zlib/deflate.c:320:27
#2 0x4a2a24 in test_dict_deflate /src/example_dict_fuzzer.c:61:11
#3 0x4a3e9b in LLVMFuzzerTestOneInput /src/example_dict_fuzzer.c:156:3
#4 0x4ed04b in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /src/libfuzzer/FuzzerLoop.cpp:571:15
#5 0x4a4ff6 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) /src/libfuzzer/FuzzerDriver.cpp:280:6
#6 0x4b5e1a in fuzzer::FuzzerDriver(int*, char***, int ()(unsigned char const, unsigned long)) /src/libfuzzer/FuzzerDriver.cpp:713:9
#7 0x4a4121 in main /src/libfuzzer/FuzzerMain.cpp:20:10
#8 0x7f3d7a13b82f in __libc_start_main /build/glibc-Cl5G7W/glibc-2.23/csu/libc-start.c:291

[sebpop]

Ping. Mark, you may want to integrate this security bug fix in zlib. Do you need a testcase where the error occurs?

[sebpop]

oss-fuzz bug has been opened to the public:
https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=11360

[madler]

Yes, please provide a test case. Thanks.

[djarek]

Hi, any chance this could be merged? I found the same issue in Boost.Beast tests: https://github.com/boostorg/beast/blob/develop/test/beast/zlib/deflate_stream.cpp#L299 using valgrind (reproduces every time).

[madler]

Still looking for that test case.

[sebpop]

Bug report: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=11360

Compile with -O2 -fsanitize=memory https://github.com/google/oss-fuzz/blob/master/projects/zlib/example_dict_fuzzer.c
and the reduced input is:
Reproducer Testcase: https://oss-fuzz.com/download?testcase_id=5725547573805056
$ ./example_dict_fuzzer -timeout=25 -rss_limit_mb=2048 -runs=100 /fuzz-0

[madler]

Thanks! So that "reduced input" (514 bytes) is what is fed to result in a dependency on uninitialized memory? If so, is it fed to LLVMFuzzerTestOneInput()?

[sebpop]

Yes, this is correct. You can link the file against the stand alone driver as I did in this patch:
fc7e134#diff-b1d85ab6d47a97650d6d61de963aabbaR1
I think that patch may still apply to today's zlib.

[ltx2018]

this patch fixes a use of uninitialized value discovered by one of the fuzzers
of the oss-fuzz project:
https://github.com/google/oss-fuzz/blob/master/projects/zlib/example_dict_fuzzer.c
clang's memory sanitizer fails with:

==1==WARNING: MemorySanitizer: use-of-uninitialized-value
#0 0x5930dd in slide_hash zlib/deflate.c:222:20
#1 0x589461 in fill_window zlib/deflate.c:1558:13
#2 0x59828f in deflate_rle zlib/deflate.c:2119:13
#3 0x590614 in deflate zlib/deflate.c:1045:41
#4 0x4a2d56 in test_dict_deflate /src/example_dict_fuzzer.c:79:11
#5 0x4a3e9b in LLVMFuzzerTestOneInput /src/example_dict_fuzzer.c:156:3
#6 0x4ed04b in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /src/libfuzzer/FuzzerLoop.cpp:571:15
#7 0x4a4ff6 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) /src/libfuzzer/FuzzerDriver.cpp:280:6
#8 0x4b5e1a in fuzzer::FuzzerDriver(int*, char***, int ()(unsigned char const, unsigned long)) /src/libfuzzer/FuzzerDriver.cpp:713:9
#9 0x4a4121 in main /src/libfuzzer/FuzzerMain.cpp:20:10
#10 0x7f3d7a13b82f in __libc_start_main /build/glibc-Cl5G7W/glibc-2.23/csu/libc-start.c:291
#11 0x41ed08 in start Uninitialized value was created by a heap allocation #0 0x45fa10 in malloc /src/llvm/projects/compiler-rt/lib/msan/msan_interceptors.cc:911 #1 0x586920 in deflateInit2 zlib/deflate.c:320:27
#2 0x4a2a24 in test_dict_deflate /src/example_dict_fuzzer.c:61:11
#3 0x4a3e9b in LLVMFuzzerTestOneInput /src/example_dict_fuzzer.c:156:3
#4 0x4ed04b in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /src/libfuzzer/FuzzerLoop.cpp:571:15
#5 0x4a4ff6 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) /src/libfuzzer/FuzzerDriver.cpp:280:6
#6 0x4b5e1a in fuzzer::FuzzerDriver(int*, char***, int ()(unsigned char const, unsigned long)) /src/libfuzzer/FuzzerDriver.cpp:713:9
#7 0x4a4121 in main /src/libfuzzer/FuzzerMain.cpp:20:10
#8 0x7f3d7a13b82f in __libc_start_main /build/glibc-Cl5G7W/glibc-2.23/csu/libc-start.c:291

I got the same issue. why this request still not merged？

[Adenilson]

I confirm the fix works (and that the issue is real), running on linux@aarch64:
a) ToT:
adenilson@rock64:~/canonical-fork/build$ valgrind ./example
==21597== Memcheck, a memory error detector
==21597== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
==21597== Using Valgrind-3.12.0.SVN and LibVEX; rerun with -h for copyright info
==21597== Command: ./example
==21597==
zlib version 1.2.11.1-motley = 0x12b1, compile flags = 0xa9
uncompress(): hello, hello!
gzread(): hello, hello!
gzgets() after gzseek: hello!
inflate(): hello, hello!
==21597== Conditional jump or move depends on uninitialised value(s)
==21597== at 0x4863A28: slide_hash (in /home/adenilson/canonical-fork/build/libz.so.1.2.11.1-motley)
==21597==
large_inflate(): OK
after inflateSync(): hello, hello!
inflate with dictionary: hello, hello!
==21597==
==21597== HEAP SUMMARY:
==21597== in use at exit: 0 bytes in 0 blocks
==21597== total heap usage: 50 allocs, 50 frees, 1,880,510 bytes allocated
==21597==
==21597== All heap blocks were freed -- no leaks are possible
==21597==
==21597== For counts of detected and suppressed errors, rerun with: -v
==21597== Use --track-origins=yes to see where uninitialised values come from
==21597== ERROR SUMMARY: 7266 errors from 1 contexts (suppressed: 0
from 0)

b) Patched (memset)
adenilson@rock64:~/canonical-fork/build$ valgrind ./example
==21703== Memcheck, a memory error detector
==21703== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
==21703== Using Valgrind-3.12.0.SVN and LibVEX; rerun with -h for copyright info
==21703== Command: ./example
==21703==
zlib version 1.2.11.1-motley = 0x12b1, compile flags = 0xa9
uncompress(): hello, hello!
gzread(): hello, hello!
gzgets() after gzseek: hello!
inflate(): hello, hello!
large_inflate(): OK
after inflateSync(): hello, hello!
inflate with dictionary: hello, hello!
==21703==
==21703== HEAP SUMMARY:
==21703== in use at exit: 0 bytes in 0 blocks
==21703== total heap usage: 50 allocs, 50 frees, 1,880,510 bytes allocated
==21703==
==21703== All heap blocks were freed -- no leaks are possible
==21703==
==21703== For counts of detected and suppressed errors, rerun with: -v
==21703== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

[ProgramMax]

Noel Gordon from Google suggested the memset() should perhaps be zmemzero() here. I agree.

Really glad fuzzing caught this and you all made a patch and repro case. Thank you!

[Adenilson]

Final fix landed on Chromium can be found at:
https://chromium-review.googlesource.com/c/chromium/src/+/2016772

[hedsnz]

@madler Could you please merge this?

[madler]

No, I need to find a gentler fix.

My clang (macOS) doesn't support the memory sanitizer. I'll look for one that does.

[madler]

deflate is working as intended and correctly in this case. From the source code:

        /* If n is not on any hash chain, prev[n] is garbage but
         * its value will never be used.
         */

I applied another pull request that turns MSAN off in the slide_hash() function. See 981ee75

[Neustradamus]

Linked to:

• slide_hash: add MSAN annotation to suppress known read from uninitialised memory #561