Opened 9 months ago

Last modified 9 months ago

#33140 new defect

Clusterfuzz environment flags reused for dependencies

Reported by: cypherpunks Owned by:
Priority: Medium Milestone:
Component: Core Tor Version:
Severity: Normal Keywords: clusterfuzz, oss-fuzz
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

The build script for tor at oss-fuzz currently reuses clusterfuzz environment variables to compile dependencies. This has consequences when the dependencies themselves are upstream projects at oss-fuzz. The build environment sets the following flags to enable fuzzing of a target project:

CC=clang
CXX=clang++
CFLAGS=-O1 -fno-omit-frame-pointer -gline-tables-only -DFUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION -fsanitize=address -fsanitize-address-use-after-scope -fsanitize=fuzzer-no-link
CXXFLAGS=-O1 -fno-omit-frame-pointer -gline-tables-only -DFUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION -fsanitize=address -fsanitize-address-use-after-scope -fsanitize=fuzzer-no-link -stdlib=libc++

In the case of zlib: Using the environment flags above as-is results in activating oss-fuzz instrumentation. Eventually resulting in ambiguously placed undefined symbol __sancov_lowest_stack because stack depth tracing was not instrumented properly. Which leads to a rabbit-hole of why are we fixing instrumenting fuzzers in tor's dependencies?

Now Openssl also has an upstream clusterfuzz instance and so leaving the environment flags as-is also results in instrumenting openssl for oss-fuzz.

This sounds wrong. If we're fuzzing tor then why are we also instrumenting dependencies for clusterfuzz? It looks like the dependencies should override these flags when built to avoid conflicts.

When the flags are overridden to build debug dependencies, followed by building tor's fuzzers as usual, check_build tor passes all tests.

Child Tickets

Change History (2)

comment:1 Changed 9 months ago by nickm

This sounds wrong. If we're fuzzing tor then why are we also instrumenting dependencies for clusterfuzz? It looks like the dependencies should override these flags when built to avoid conflicts.

I'm no fuzzing expert, but here is my understanding:

I think we want to instrument everything, so that we can find it when code outside of Tor is caused by Tor to leak memory, invoke undefined behavior, or whatever.

Even though openssl is fuzzed itself, that's no guarantee that Tor is using openssl correctly: we might be invoking an openssl function with a too-short buffer, or using it with an uninitialized object. If we did, then the fuzzers might not find that unless the openssl code that we're using is also implemented.

comment:2 Changed 9 months ago by cypherpunks

If tor doesn't use openssl correctly tor fails the test. Openssl is fuzzed upstream so the instrumentation doesn't help because we don't complete the openssl fuzz build, we complete the tor fuzz build.

That and when used with zlib you literally have no choice but to fully instrument the zlib build or expect zlib to to break eventually.

I'm going to leave this here for open discussion and check in periodically.

edit: see https://google.github.io/oss-fuzz/advanced-topics/ideal-integration/#build-support

That's the only time the flags are mentioned in oss-fuzz docs. It reinforces that the environment flags are for the fuzzing entry point, which is tor.

Last edited 9 months ago by cypherpunks (previous) (diff)
Note: See TracTickets for help on using tickets.