This is an archive of the discontinued LLVM Phabricator instance.

[libfuzzer] Fix UB when calculating Log(0) in StackDepthStepFunction().
ClosedPublic

Authored by Dor1s on Dec 20 2017, 11:06 AM.

Details

Summary

__builtin_clz used for Log calculation returns an undefined result
when argument is 0. I noticed that issue when was testing some fuzzers:

/src/libfuzzer/FuzzerTracePC.h:282:33: runtime error: shift exponent 450349 is too large for 32-bit type 'uint32_t' (aka 'unsigned int')
  #0 0x43d83f in operator() /src/libfuzzer/FuzzerTracePC.h:283:33
  #1 0x43d83f in void fuzzer::TracePC::CollectFeatures<fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool*)::$_1>(fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool*)::$_1) const /src/libfuzzer/FuzzerTracePC.h:290
  #2 0x43cbd4 in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool*) /src/libfuzzer/FuzzerLoop.cpp:445:7
  #3 0x43e5f1 in fuzzer::Fuzzer::ReadAndExecuteSeedCorpora(std::__1::vector<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, fuzzer::fuzzer_allocator<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > > > const&) /src/libfuzzer/FuzzerLoop.cpp:706:5
  #4 0x43e9e1 in fuzzer::Fuzzer::Loop(std::__1::vector<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, fuzzer::fuzzer_allocator<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > > > const&) /src/libfuzzer/FuzzerLoop.cpp:739:3
  #5 0x432f8c in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /src/libfuzzer/FuzzerDriver.cpp:754:6
  #6 0x42ee18 in main /src/libfuzzer/FuzzerMain.cpp:20:10
  #7 0x7f17ffeb182f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)
  #8 0x407838 in _start (/out/rotate_fuzzer+0x407838)

Event Timeline

Dor1s created this revision.Dec 20 2017, 11:06 AM
Herald added subscribers: Restricted Project, llvm-commits. · View Herald TranscriptDec 20 2017, 11:06 AM
Dor1s edited the summary of this revision. (Show Details)Dec 20 2017, 11:08 AM
kcc accepted this revision.Dec 20 2017, 11:10 AM

LGTM

This revision is now accepted and ready to land.Dec 20 2017, 11:10 AM
Dor1s added inline comments.Dec 20 2017, 11:12 AM
lib/fuzzer/FuzzerTracePC.h
279

I guess we can do if (A < 8) return A here and remove another check from line #281.

Dor1s added inline comments.Dec 20 2017, 11:32 AM
lib/fuzzer/FuzzerTracePC.h
279

or maybe even 1 << 3 instead of 8. Either way, landing what you've approved. Thanks!

This revision was automatically updated to reflect the committed changes.