ghidra icon indicating copy to clipboard operation
ghidra copied to clipboard

Published 20 hours ago • verified publisher icon NationalSecurityAgency

Fix UBSAN errors in decompiler

Open tetsuo-cpp opened this issue 3 years ago • 3 comments

When compiling a x86.slaspec to a .sla file, I noticed that UBSAN reports a few errors.

tetsuo@Alexs-MacBook-Pro maat % UBSAN_OPTIONS=print_stacktrace=1 ./build/sanitize/sleigh/sleigh_opt ./build/sanitize/_deps/ghidrasource-src/Ghidra/Processors/x86/data/languages/x86.slaspec test.sla
/Users/tetsuo/COde/maat/build/sanitize/_deps/ghidrasource-src/Ghidra/Features/Decompiler/src/decompile/cpp/address.cc:649:17: runtime error: left shift of negative value -1
    #0 0x1044c6008 in zero_extend(long long&, int) address.cc:649
    #1 0x104636794 in TokenField::maxValue() const slghpatexpress.hh:103
    #2 0x1046c685c in SleighCompile::attachVarnodes(std::__1::vector<SleighSymbol*, std::__1::allocator<SleighSymbol*> >*, std::__1::vector<SleighSymbol*, std::__1::allocator<SleighSymbol*> >*) slgh_compile.cc:2720
    #3 0x104720244 in yyparse() slghparse.y:237
    #4 0x1046d6ab0 in SleighCompile::run_compilation(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&) slgh_compile.cc:3543
    #5 0x1046d9e18 in main slgh_compile.cc:3819
    #6 0x104cdd0f0 in start+0x204 (dyld:arm64e+0x50f0)

SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /Users/tetsuo/COde/maat/build/sanitize/_deps/ghidrasource-src/Ghidra/Features/Decompiler/src/decompile/cpp/address.cc:649:17 in
/Users/tetsuo/COde/maat/build/sanitize/_deps/ghidrasource-src/Ghidra/Features/Decompiler/src/decompile/cpp/address.cc:650:8: runtime error: left shift of negative value -4
    #0 0x1044c5fc8 in zero_extend(long long&, int) address.cc:650
    #1 0x104636794 in TokenField::maxValue() const slghpatexpress.hh:103
    #2 0x1046c685c in SleighCompile::attachVarnodes(std::__1::vector<SleighSymbol*, std::__1::allocator<SleighSymbol*> >*, std::__1::vector<SleighSymbol*, std::__1::allocator<SleighSymbol*> >*) slgh_compile.cc:2720
    #3 0x104720244 in yyparse() slghparse.y:237
    #4 0x1046d6ab0 in SleighCompile::run_compilation(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&) slgh_compile.cc:3543
    #5 0x1046d9e18 in main slgh_compile.cc:3819
    #6 0x104cdd0f0 in start+0x204 (dyld:arm64e+0x50f0)

SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /Users/tetsuo/COde/maat/build/sanitize/_deps/ghidrasource-src/Ghidra/Features/Decompiler/src/decompile/cpp/address.cc:650:8 in
/Users/tetsuo/COde/maat/build/sanitize/_deps/ghidrasource-src/Ghidra/Features/Decompiler/src/decompile/cpp/slgh_compile.cc:2153:24: runtime error: downcast of address 0x000108b00ca0 which does not point to an object of type 'LabelSymbol'
0x000108b00ca0: note: object is of type 'OperandSymbol'
 00 00 00 00  70 45 81 04 01 00 00 00  72 65 67 38 00 be be be  be be be be be be be be  be be be be
              ^~~~~~~~~~~~~~~~~~~~~~~
              vptr for 'OperandSymbol'
    #0 0x1046b905c in SleighCompile::checkSymbols(SymbolScope*) slgh_compile.cc:2153
    #1 0x1046d1520 in SleighCompile::finalizeSections(Constructor*, SectionVector*) slgh_compile.cc:3212
    #2 0x1046d4ee8 in SleighCompile::buildConstructor(Constructor*, PatternEquation*, std::__1::vector<ContextChange*, std::__1::allocator<ContextChange*> >*, SectionVector*) slgh_compile.cc:3463
    #3 0x104719de0 in yyparse() slghparse.y
    #4 0x1046d6ab0 in SleighCompile::run_compilation(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&) slgh_compile.cc:3543
    #5 0x1046d9e18 in main slgh_compile.cc:3819
    #6 0x104cdd0f0 in start+0x204 (dyld:arm64e+0x50f0)

SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /Users/tetsuo/COde/maat/build/sanitize/_deps/ghidrasource-src/Ghidra/Features/Decompiler/src/decompile/cpp/slgh_compile.cc:2153:24 in
/Users/tetsuo/COde/maat/build/sanitize/_deps/ghidrasource-src/Ghidra/Features/Decompiler/src/decompile/cpp/slgh_compile.cc:2154:14: runtime error: member call on address 0x000108b00ca0 which does not point to an object of type 'LabelSymbol'
0x000108b00ca0: note: object is of type 'OperandSymbol'
 00 00 00 00  70 45 81 04 01 00 00 00  72 65 67 38 00 be be be  be be be be be be be be  be be be be
              ^~~~~~~~~~~~~~~~~~~~~~~
              vptr for 'OperandSymbol'
    #0 0x1046b8d88 in SleighCompile::checkSymbols(SymbolScope*) slgh_compile.cc:2154
    #1 0x1046d1520 in SleighCompile::finalizeSections(Constructor*, SectionVector*) slgh_compile.cc:3212
    #2 0x1046d4ee8 in SleighCompile::buildConstructor(Constructor*, PatternEquation*, std::__1::vector<ContextChange*, std::__1::allocator<ContextChange*> >*, SectionVector*) slgh_compile.cc:3463
    #3 0x104719de0 in yyparse() slghparse.y
    #4 0x1046d6ab0 in SleighCompile::run_compilation(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&) slgh_compile.cc:3543
    #5 0x1046d9e18 in main slgh_compile.cc:3819
    #6 0x104cdd0f0 in start+0x204 (dyld:arm64e+0x50f0)

SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /Users/tetsuo/COde/maat/build/sanitize/_deps/ghidrasource-src/Ghidra/Features/Decompiler/src/decompile/cpp/slgh_compile.cc:2154:14 in
/Users/tetsuo/COde/maat/build/sanitize/_deps/ghidrasource-src/Ghidra/Features/Decompiler/src/decompile/cpp/semantics.hh:51:75: runtime error: load of value 24, which is not a valid value for type 'ConstTpl::v_field'
    #0 0x104678bac in ConstTpl::ConstTpl(ConstTpl const&) semantics.hh:51
    #1 0x10466dac4 in HandleTpl::HandleTpl(ConstTpl const&, ConstTpl const&, VarnodeTpl const*, AddrSpace*, unsigned long long) semantics.cc:580
    #2 0x1046c9994 in SleighCompile::setResultStarVarnode(ConstructTpl*, StarQuality*, VarnodeTpl*) slgh_compile.cc:2897
    #3 0x10471d8b0 in yyparse() slghparse.y:350
    #4 0x1046d6ab0 in SleighCompile::run_compilation(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&) slgh_compile.cc:3543
    #5 0x1046d9e18 in main slgh_compile.cc:3819
    #6 0x104cdd0f0 in start+0x204 (dyld:arm64e+0x50f0)

SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /Users/tetsuo/COde/maat/build/sanitize/_deps/ghidrasource-src/Ghidra/Features/Decompiler/src/decompile/cpp/semantics.hh:51:75 in
/Users/tetsuo/COde/maat/build/sanitize/_deps/ghidrasource-src/Ghidra/Features/Decompiler/src/decompile/cpp/pcodecompile.cc:626:36: runtime error: shift exponent 64 is too large for 64-bit type 'unsigned long long'
    #0 0x10456a6f4 in PcodeCompile::assignBitRange(VarnodeTpl*, unsigned int, unsigned int, ExprTree*) pcodecompile.cc:626
    #1 0x104712798 in yyparse() slghparse.y:367
    #2 0x1046d6ab0 in SleighCompile::run_compilation(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&) slgh_compile.cc:3543
    #3 0x1046d9e18 in main slgh_compile.cc:3819
    #4 0x104cdd0f0 in start+0x204 (dyld:arm64e+0x50f0)

SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /Users/tetsuo/COde/maat/build/sanitize/_deps/ghidrasource-src/Ghidra/Features/Decompiler/src/decompile/cpp/pcodecompile.cc:626:36 in
/Users/tetsuo/COde/maat/build/sanitize/_deps/ghidrasource-src/Ghidra/Features/Decompiler/src/decompile/cpp/pcodecompile.cc:731:16: runtime error: shift exponent 127 is too large for 64-bit type 'uintb' (aka 'unsigned long long')
    #0 0x10456b780 in PcodeCompile::createBitRange(SpecificSymbol*, unsigned int, unsigned int) pcodecompile.cc:731
    #1 0x10471c494 in yyparse() slghparse.y:446
    #2 0x1046d6ab0 in SleighCompile::run_compilation(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&) slgh_compile.cc:3543
    #3 0x1046d9e18 in main slgh_compile.cc:3819
    #4 0x104cdd0f0 in start+0x204 (dyld:arm64e+0x50f0)

SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /Users/tetsuo/COde/maat/build/sanitize/_deps/ghidrasource-src/Ghidra/Features/Decompiler/src/decompile/cpp/pcodecompile.cc:731:16 in
/Users/tetsuo/COde/maat/build/sanitize/_deps/ghidrasource-src/Ghidra/Features/Decompiler/src/decompile/cpp/pcodecompile.cc:626:18: runtime error: shift exponent 127 is too large for 64-bit type 'uintb' (aka 'unsigned long long')
    #0 0x104569fec in PcodeCompile::assignBitRange(VarnodeTpl*, unsigned int, unsigned int, ExprTree*) pcodecompile.cc:626
    #1 0x104712798 in yyparse() slghparse.y:367
    #2 0x1046d6ab0 in SleighCompile::run_compilation(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&) slgh_compile.cc:3543
    #3 0x1046d9e18 in main slgh_compile.cc:3819
    #4 0x104cdd0f0 in start+0x204 (dyld:arm64e+0x50f0)

SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /Users/tetsuo/COde/maat/build/sanitize/_deps/ghidrasource-src/Ghidra/Features/Decompiler/src/decompile/cpp/pcodecompile.cc:626:18 in
WARN  221 NOP constructors found
WARN  Use -n switch to list each individually
WARN  202 unnecessary extensions/truncations were converted to copies
WARN  Use -u switch to list each individually
WARN  10 operations wrote to temporaries that were not read
WARN  Use -t switch to list each individually

tetsuo-cpp avatar Feb 06 '22 15:02 tetsuo-cpp

I was messing with scan-build make test this morning and it was picking up a few things as well (the ConstTpl initializers were the only overlap). Should look at -fsanitize=undefined with make test as well.

https://gist.github.com/mumbel/bf5edf310f85cc187c6954dfd7c0cd90

Having some issues building with BFD. This is still on my todo list.

  • [x] Fix UB uncovered by make test

tetsuo-cpp avatar Feb 07 '22 09:02 tetsuo-cpp

@tetsuo-cpp here is the one I've been messing around with https://gist.github.com/mumbel/7bfff8a7831e6289d68bec13eb51665c

I switched gcc out for clang, added a few extra flags and propagated the flags through all the CXX. The BFD thing is the ADDITIONAL_FLAGS, haven't ever looked how the project is actually getting around that #error

mumbel avatar Feb 07 '22 14:02 mumbel

@mumbel Ok, make check seems to be working ok with -fsanitize=undefined now. Sorry for the delay, I believe this is good to go.

tetsuo-cpp avatar Jul 05 '22 06:07 tetsuo-cpp

Please let me know if you have any interest in getting this in and I'm happy to resolve any merge conflicts.

tetsuo-cpp avatar Jul 25 '23 03:07 tetsuo-cpp