search

google / oss-fuzz

OSS-Fuzz - continuous fuzzing for open source software.
https://google.github.io/oss-fuzz
Apache License 2.0
10.45k stars 2.21k forks source link

fuzzer build failure for upx is not debuggable #11639

Open jreiser opened 7 months ago

jreiser commented 7 months ago

https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=66785&q=label%3AProj-upx The log ends with

Step #3 - "compile-afl-address-x86_64": ---------------------------------------------------------------
Step #3 - "compile-afl-address-x86_64": Copying precompiled AFL++
Step #3 - "compile-afl-address-x86_64": 
Step #3 - "compile-afl-address-x86_64": AFL++ target compilation setup:
Step #3 - "compile-afl-address-x86_64": AFL_IGNORE_UNKNOWN_ENVS=1
Step #3 - "compile-afl-address-x86_64": AFL_QUIET=1
Step #3 - "compile-afl-address-x86_64": AFL_IGNORE_PROBLEMS=1
Step #3 - "compile-afl-address-x86_64": 
Step #3 - "compile-afl-address-x86_64":  done.
Step #3 - "compile-afl-address-x86_64": ---------------------------------------------------------------
Step #3 - "compile-afl-address-x86_64": CC=/src/aflplusplus/afl-clang-fast
Step #3 - "compile-afl-address-x86_64": CXX=/src/aflplusplus/afl-clang-fast++
Step #3 - "compile-afl-address-x86_64": CFLAGS=-O1 -fno-omit-frame-pointer -gline-tables-only -DFUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION -fsanitize=address -fsanitize-address-use-after-scope 
Step #3 - "compile-afl-address-x86_64": CXXFLAGS=-O1 -fno-omit-frame-pointer -gline-tables-only -DFUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION -fsanitize=address -fsanitize-address-use-after-scope  -stdlib=libc++
Step #3 - "compile-afl-address-x86_64": RUSTFLAGS=--cfg fuzzing -Zsanitizer=address -Cdebuginfo=1 -Cforce-frame-pointers
Step #3 - "compile-afl-address-x86_64": ---------------------------------------------------------------
Step #3 - "compile-afl-address-x86_64": + sed -i 's/ \&\& __clang_major__ < 15//m' /src/upx/src/util/util.cpp
Step #3 - "compile-afl-address-x86_64": + git apply /src/upx/fuzzers/build.patch
Step #3 - "compile-afl-address-x86_64": error: patch failed: CMakeLists.txt:595
Step #3 - "compile-afl-address-x86_64": error: CMakeLists.txt: patch does not apply
Step #3 - "compile-afl-address-x86_64": ********************************************************************************
Step #3 - "compile-afl-address-x86_64": Failed to build.
Step #3 - "compile-afl-address-x86_64": To reproduce, run:
Step #3 - "compile-afl-address-x86_64": python infra/helper.py build_image upx
Step #3 - "compile-afl-address-x86_64": python infra/helper.py build_fuzzers --sanitizer address --engine afl --architecture x86_64 upx
Step #3 - "compile-afl-address-x86_64": ********************************************************************************
Finished Step #3 - "compile-afl-address-x86_64"
ERROR
ERROR: build step 3 "gcr.io/cloud-builders/docker" failed: step exited with non-zero status: 1

and yes, the instructions "To reproduce, run:" do reproduce the error. However, there is little clue about how to fix it. There should be relevant data and suggestions.

If a patch does not apply, then there should be a .rej reject file which gives the hunk that failed to apply: a path to the file from filesystem root, the line numbers, the actual pre-patch context in the file, the hunk itself. This information is crucial to understand why the patch does not apply.

Looking for a build.patch file:

$ find . -name build.patch
./projects/lucene/build.patch
./projects/upx/fuzzers/build.patch
./projects/retrofit/build.patch
./projects/spring-webflow/build.patch
./projects/jxls/build.patch
./build/out/upx/src/upx/fuzzers/build.patch

The file ./projects/upx/build.shcontains the text

# Temporary fix for clang bug of upx
sed -i 's/ \&\& __clang_major__ < 15//m' /src/upx/src/util/util.cpp
git apply $SRC/upx/fuzzers/build.patch

but what is the meaning of clang bug of upx Does upx have a bug, or does clang have a bug, or both? And what is the nature of the bug? Also, the command-line portion //m is too cryptic; it should be explained. (man sed says nothing about a m trailing modifier to the s command of sed.

Looking at git blame of tip of branch devel of util/util.cpp:

62dbf8485 src/util/util.cpp (Markus F.X.J. Oberhumer 2023-09-04 07:28:48 +0200 279) // much better memswap(), optimized for our use case in sort functions below
718ec468f src/util/util.cpp (Markus F.X.J. Oberhumer 2024-02-02 10:17:16 +0100 280) static void memswap_no_overlap(byte *a, byte *b, size_t n) noexcept {
62dbf8485 src/util/util.cpp (Markus F.X.J. Oberhumer 2023-09-04 07:28:48 +0200 281) #if defined(__clang__) && __clang_major__ < 15
62dbf8485 src/util/util.cpp (Markus F.X.J. Oberhumer 2023-09-04 07:28:48 +0200 282)     // work around a clang < 15 ICE (Internal Compiler Error)
632c7c482 src/util/util.cpp (Markus F.X.J. Oberhumer 2023-10-05 03:51:27 +0200 283)     // @COMPILER_BUG @CLANG_BUG
bb6b08786 src/util/util.cpp (Markus F.X.J. Oberhumer 2023-09-01 10:49:47 +0200 284)     upx_memswap(a, b, n);

Perhaps the comment and patch in ./projects/upx/build.sh is unnecessary?

Anyway, the problem of the fuzzer not building is not the fault of any contents in the source of upx.

DavidKorczynski commented 6 months ago

I don't think this is an OSS-Fuzz issue as such because it seems specific to the upx-project integration (i.e. the build.sh is considered to be owned by the upstream project from an OSS-Fuzz perspective). It seems as a consequence of the integration being carried out by a 3rd party security researcher, similarly to described here: https://github.com/google/oss-fuzz/issues/11537#issuecomment-1915376479

Is there any chance you will be able to maintain the logic in https://github.com/google/oss-fuzz/tree/master/projects/upx @jreiser ? We could e.g. move as much logic as possible to upstream UPX so that you have direct access to adjusting the build/fuzz logic.

jreiser commented 6 months ago

On 4/9/24 16:19, DavidKorczynski wrote:

I don't think this is an OSS-Fuzz issue as such because it seems specific to the upx-project integration (i.e. the |build.sh| is considered to be owned by the upstream project from an OSS-Fuzz perspective). It seems as a consequence of the integration being carried out by a 3rd party security researcher, similarly to described here:

11537 (comment)

https://github.com/google/oss-fuzz/issues/11537#issuecomment-1915376479

Is there any chance you will be able to maintain the logic in https://github.com/google/oss-fuzz/tree/master/projects/upx https://github.com/google/oss-fuzz/tree/master/projects/upx @jreiser https://github.com/jreiser ? We could e.g. move as much logic as possible to upstream UPX so that you have direct access to adjusting the build/fuzz logic.

Yes. Please give me commit access to the git source for https://github.com/google/oss-fuzz/blob/master/projects/upx/build.sh

so that I can remove (beginning at line 18):

Temporary fix for clang bug of upx

sed -i 's/ \&\& __clang_major__ < 15//m' /src/upx/src/util/util.cpp
git apply $SRC/upx/fuzzers/build.patch

which is no longer applicable, and fix other similar problems soonest.

I am displeased that so far it has taken 6 weeks to perform effective triage on this issue, instead of a couple days or less. The directions given to me said "file a bug report at OSS-fuzz" if there are issues with fuzzing. OSS-fuzz dropped the ball by not immediately re-directing my report to someone who could fix it or enlist my help to get further information. LeviathanSecurity dropped the ball by not monitoring its fuzzing projects daily. Technology projects actually require management.

-- John

— Reply to this email directly, view it on GitHub https://github.com/google/oss-fuzz/issues/11639#issuecomment-2046188320, or unsubscribe https://github.com/notifications/unsubscribe-auth/AFC7ZTIZMUPHMFWOCZAMG6LY4RZRPAVCNFSM6AAAAABD2QQVIKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANBWGE4DQMZSGA. You are receiving this because you were mentioned.Message ID: @.***>

jonathanmetzman commented 5 months ago

Sorry this has been getting slow replies. You can just submit a pull request to change https://github.com/google/oss-fuzz/blob/master/projects/upx/build.sh and we will merge it. If that seems too bureaucratic for you, you can change the dockerfile to use a build.sh from the upx repo.

jreiser commented 5 months ago

Please approve and apply Pull Request: https://github.com/google/oss-fuzz/pull/11896

-- John

On 5/2/24 12:43, jonathanmetzman wrote:

Sorry this has been getting slow replies. You can just submit a pull request to change https://github.com/google/oss-fuzz/blob/master/projects/upx/build.sh https://github.com/google/oss-fuzz/blob/master/projects/upx/build.sh and we will merge it. If that seems too bureaucratic for you, you can change the dockerfile to use a build.sh from the upx repo.

— Reply to this email directly, view it on GitHub https://github.com/google/oss-fuzz/issues/11639#issuecomment-2091419473, or unsubscribe https://github.com/notifications/unsubscribe-auth/AFC7ZTPRETZZU323SN3UZ4DZAKJMTAVCNFSM6AAAAABD2QQVIKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAOJRGQYTSNBXGM. You are receiving this because you were mentioned.Message ID: @.***>