TrueBitProject / lanai

31 stars 7 forks source link

Harden against memory corruption #2

Open tgoddard opened 8 years ago

tgoddard commented 8 years ago

Hello,

I put the interpreter through AFL to find memory corruptions issues early on, set the foundations. Quite a few popped out - need to harden that up before allowing people to run untrusted code.

Identified the following causes so far:

Here are stack traces caused by each of those issues, attached the raw test cases:

id:000001,sig:06,src:000000,op:flip1,pos:33
    #0 0x804ed7e in Linker::link(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&) /home/aura/dev/lanai/interpreter/linker.cpp:89
    #1 0x804a5d7 in main /home/aura/dev/lanai/interpreter/main.cpp:39
    #2 0xb6e595f6 in __libc_start_main (/lib/i386-linux-gnu/libc.so.6+0x185f6)
    #3 0x804b960  (/home/aura/dev/lanai/interpreter/lanai-int+0x804b960)

id:000004,sig:06,src:000000,op:flip1,pos:50
    #0 0x804edd5 in Linker::link(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&) /home/aura/dev/lanai/interpreter/linker.cpp:117
    #1 0x804a5d7 in main /home/aura/dev/lanai/interpreter/main.cpp:39
    #2 0xb6dfa5f6 in __libc_start_main (/lib/i386-linux-gnu/libc.so.6+0x185f6)
    #3 0x804b960  (/home/aura/dev/lanai/interpreter/lanai-int+0x804b960)

id:000009,sig:06,src:000000,op:flip1,pos:53
    #0 0x80616cf in Interpreter::memStoreWord(unsigned int, unsigned int) /home/aura/dev/lanai/interpreter/interpreter.cpp:484
    #1 0x80616cf in Interpreter::run(unsigned int, unsigned int, bool) /home/aura/dev/lanai/interpreter/interpreter.cpp:92
    #2 0x804b0d9 in main /home/aura/dev/lanai/interpreter/main.cpp:47
    #3 0xb6e5a5f6 in __libc_start_main (/lib/i386-linux-gnu/libc.so.6+0x185f6)
    #4 0x804b960  (/home/aura/dev/lanai/interpreter/lanai-int+0x804b960)
    #0 0xb729f054 in operator new(unsigned int) (/usr/lib/i386-linux-gnu/libasan.so.3+0xbf054)
    #1 0xb7170beb in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_create(unsigned int&, unsigned int) (/usr/lib/i386-linux-gnu/libstdc++.so.6+0x109beb)

id:000080,sig:06,src:000000,op:int32,pos:52,val:be:-32769
    #0 0x80602fc in Interpreter::memStoreByte(unsigned int, unsigned int) /home/aura/dev/lanai/interpreter/interpreter.cpp:490
    #1 0x80602fc in Interpreter::run(unsigned int, unsigned int, bool) /home/aura/dev/lanai/interpreter/interpreter.cpp:199
    #2 0x804b0d9 in main /home/aura/dev/lanai/interpreter/main.cpp:47
    #3 0xb6dfd5f6 in __libc_start_main (/lib/i386-linux-gnu/libc.so.6+0x185f6)
    #4 0x804b960  (/home/aura/dev/lanai/interpreter/lanai-int+0x804b960)
    #0 0xb7242054 in operator new(unsigned int) (/usr/lib/i386-linux-gnu/libasan.so.3+0xbf054)
    #1 0xb7113beb in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_create(unsigned int&, unsigned int) (/usr/lib/i386-linux-gnu/libstdc++.so.6+0x109beb)

crashes.zip

chriseth commented 8 years ago

Thanks a lot!

Yeah, this is just a crude "will it work at all" implementation, but you are right, usually you never do a proper reimplementation from scratch, so better to keep the quality high even in hacky prototypes.

The linker certainly has to be improved, though I would prefer statically linked binaries directly from llvm - I could not find a proper specification of the lanai elf format and had to do some guesswork in some places.

I'm kind of wondering about the memStoreWord and memStoreByte functions, though, as there are bounds checks. Perhaps this is about the 32 bit pos + 4 operation?

tgoddard commented 8 years ago

That could well be the cause if that overflows - would suddenly become very small and not resize the string.