search

pistacheio / pistache

A high-performance REST toolkit written in C++
https://pistacheio.github.io/pistache/
Apache License 2.0
3.17k stars 698 forks source link

Fix out of buffer read in StreamCursor while parsing HTTP request #1192

Closed tyler92 closed 7 months ago

tyler92 commented 7 months ago

This issue has been found as a result of fuzzing https://github.com/pistacheio/pistache/pull/1191

AddressSanitizer report ==2032182==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x506000aa5fe0 at pc 0x55739b8e848d bp 0x7ffcc86b6550 sp 0x7ffcc86b6548 READ of size 1 at 0x506000aa5fe0 thread T0 #0 0x55739b8e848c in Pistache::StreamBuf::snext() const pistache/src/../include/pistache/stream.h:62:20 #1 0x55739b8e823f in Pistache::StreamCursor::next() const pistache/src/common/stream.cc:186:21 #2 0x55739b8e80f7 in Pistache::StreamCursor::eol() const pistache/src/common/stream.cc:177:67 #3 0x55739b756857 in Pistache::Http::Private::RequestLineStep::apply(Pistache::StreamCursor&) pistache/src/common/http.cc:235:28 #4 0x55739b75d81a in Pistache::Http::Private::ParserBase::parse() pistache/src/common/http.cc:545:36 #5 0x55739b724983 in fuzz_request_parser(std::__cxx11::basic_string, std::allocator> const&)::$_0::operator()() const pistache/tests/fuzzers/fuzz_parser.cpp:82:48 #6 0x55739b7126c4 in void ignoreExceptions, std::allocator> const&)::$_0>(fuzz_request_parser(std::__cxx11::basic_string, std::allocator> const&)::$_0 const&) pistache/tests/fuzzers/fuzz_parser.cpp:17:9 #7 0x55739b71238c in fuzz_request_parser(std::__cxx11::basic_string, std::allocator> const&) pistache/tests/fuzzers/fuzz_parser.cpp:82:9 #8 0x55739b7141c3 in LLVMFuzzerTestOneInput pistache/tests/fuzzers/fuzz_parser.cpp:153:9 #9 0x55739b6198f0 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) (pistache/build/tests/fuzzers/fuzz_parser+0x13a8f0) (BuildId: 77a710c1e69da7f096aec08fd6fb4b1fc2e8e9e4) #10 0x55739b619065 in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool, bool*) (pistache/build/tests/fuzzers/fuzz_parser+0x13a065) (BuildId: 77a710c1e69da7f096aec08fd6fb4b1fc2e8e9e4) #11 0x55739b61a845 in fuzzer::Fuzzer::MutateAndTestOne() (pistache/build/tests/fuzzers/fuzz_parser+0x13b845) (BuildId: 77a710c1e69da7f096aec08fd6fb4b1fc2e8e9e4) #12 0x55739b61b455 in fuzzer::Fuzzer::Loop(std::vector>&) (pistache/build/tests/fuzzers/fuzz_parser+0x13c455) (BuildId: 77a710c1e69da7f096aec08fd6fb4b1fc2e8e9e4) #13 0x55739b6095bb in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) (pistache/build/tests/fuzzers/fuzz_parser+0x12a5bb) (BuildId: 77a710c1e69da7f096aec08fd6fb4b1fc2e8e9e4) #14 0x55739b6325f2 in main (pistache/build/tests/fuzzers/fuzz_parser+0x1535f2) (BuildId: 77a710c1e69da7f096aec08fd6fb4b1fc2e8e9e4) #15 0x7fea27127d8f in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16 #16 0x7fea27127e3f in __libc_start_main csu/../csu/libc-start.c:392:3 #17 0x55739b5fea24 in _start (pistache/build/tests/fuzzers/fuzz_parser+0x11fa24) (BuildId: 77a710c1e69da7f096aec08fd6fb4b1fc2e8e9e4) 0x506000aa5fe0 is located 0 bytes after 64-byte region [0x506000aa5fa0,0x506000aa5fe0) allocated by thread T0 here: #0 0x55739b70b32d in operator new(unsigned long) (pistache/build/tests/fuzzers/fuzz_parser+0x22c32d) (BuildId: 77a710c1e69da7f096aec08fd6fb4b1fc2e8e9e4) #1 0x55739b795b48 in std::__new_allocator::allocate(unsigned long, void const*) /usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/new_allocator.h:137:27 #2 0x55739b7959fa in std::allocator_traits>::allocate(std::allocator&, unsigned long) /usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/alloc_traits.h:464:20 #3 0x55739b79515b in std::_Vector_base>::_M_allocate(unsigned long) /usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/stl_vector.h:378:20 #4 0x55739b79449c in void std::vector>::_M_realloc_insert(__gnu_cxx::__normal_iterator>>, char const&) /usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/vector.tcc:453:33 #5 0x55739b7940e7 in std::vector>::push_back(char const&) /usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/stl_vector.h:1287:4 #6 0x55739b793ddb in std::back_insert_iterator>>::operator=(char const&) /usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/stl_iterator.h:735:13 #7 0x55739b793bfe in std::back_insert_iterator>> std::__copy_move::__copy_m>>>(char const*, char const*, std::back_insert_iterator>>) /usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/stl_algobase.h:385:18 #8 0x55739b793965 in std::back_insert_iterator>> std::__copy_move_a2>>>(char const*, char const*, std::back_insert_iterator>>) /usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/stl_algobase.h:494:14 #9 0x55739b7934b5 in std::back_insert_iterator>> std::__copy_move_a1>>>(char const*, char const*, std::back_insert_iterator>>) /usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/stl_algobase.h:522:14 #10 0x55739b792f6a in std::back_insert_iterator>> std::__copy_move_a>>>(char const*, char const*, std::back_insert_iterator>>) /usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/stl_algobase.h:530:3 #11 0x55739b792a63 in std::back_insert_iterator>> std::copy>>>(char const*, char const*, std::back_insert_iterator>>) /usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/stl_algobase.h:619:14 #12 0x55739b75dd72 in Pistache::ArrayStreamBuf::feed(char const*, unsigned long) pistache/src/../include/pistache/stream.h:111:13 #13 0x55739b75da6a in Pistache::Http::Private::ParserBase::feed(char const*, unsigned long) pistache/src/common/http.cc:558:27 #14 0x55739b7122ad in fuzz_request_parser(std::__cxx11::basic_string, std::allocator> const&) pistache/tests/fuzzers/fuzz_parser.cpp:79:17 #15 0x55739b7141c3 in LLVMFuzzerTestOneInput pistache/tests/fuzzers/fuzz_parser.cpp:153:9 #16 0x55739b6198f0 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) (pistache/build/tests/fuzzers/fuzz_parser+0x13a8f0) (BuildId: 77a710c1e69da7f096aec08fd6fb4b1fc2e8e9e4) #17 0x55739b619065 in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool, bool*) (pistache/build/tests/fuzzers/fuzz_parser+0x13a065) (BuildId: 77a710c1e69da7f096aec08fd6fb4b1fc2e8e9e4) #18 0x55739b61a845 in fuzzer::Fuzzer::MutateAndTestOne() (pistache/build/tests/fuzzers/fuzz_parser+0x13b845) (BuildId: 77a710c1e69da7f096aec08fd6fb4b1fc2e8e9e4) #19 0x55739b61b455 in fuzzer::Fuzzer::Loop(std::vector>&) (pistache/build/tests/fuzzers/fuzz_parser+0x13c455) (BuildId: 77a710c1e69da7f096aec08fd6fb4b1fc2e8e9e4) #20 0x55739b6095bb in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) (pistache/build/tests/fuzzers/fuzz_parser+0x12a5bb) (BuildId: 77a710c1e69da7f096aec08fd6fb4b1fc2e8e9e4) #21 0x55739b6325f2 in main (pistache/build/tests/fuzzers/fuzz_parser+0x1535f2) (BuildId: 77a710c1e69da7f096aec08fd6fb4b1fc2e8e9e4) #22 0x7fea27127d8f in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16 SUMMARY: AddressSanitizer: heap-buffer-overflow pistache/src/../include/pistache/stream.h:62:20 in Pistache::StreamBuf::snext() const Shadow bytes around the buggy address: 0x506000aa5d00: fd fd fd fd fd fd fd fd fa fa fa fa fd fd fd fd 0x506000aa5d80: fd fd fd fd fa fa fa fa fd fd fd fd fd fd fd fd 0x506000aa5e00: fa fa fa fa fd fd fd fd fd fd fd fd fa fa fa fa 0x506000aa5e80: fd fd fd fd fd fd fd fd fa fa fa fa 00 00 00 00 0x506000aa5f00: 00 00 00 fa fa fa fa fa fd fd fd fd fd fd fd fd =>0x506000aa5f80: fa fa fa fa 00 00 00 00 00 00 00 00[fa]fa fa fa 0x506000aa6000: fd fd fd fd fd fd fd fd fa fa fa fa fa fa fa fa 0x506000aa6080: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd 0x506000aa6100: fa fa fa fa fd fd fd fd fd fd fd fd fa fa fa fa 0x506000aa6180: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x506000aa6200: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd Shadow byte legend (one shadow byte represents 8 application bytes): Addressable: 00 Partially addressable: 01 02 03 04 05 06 07 Heap left redzone: fa Freed heap region: fd Stack left redzone: f1 Stack mid redzone: f2 Stack right redzone: f3 Stack after return: f5 Stack use after scope: f8 Global redzone: f9 Global init order: f6 Poisoned by user: f7 Container overflow: fc Array cookie: ac Intra object redzone: bb ASan internal: fe Left alloca redzone: ca Right alloca redzone: cb ==2032182==ABORTING

Explanation: buf->in_avail() == 1 means that we can read the current character, but the next one might be out of bounds because there is no guarantee that StreamCursor works with zero-terminated string.

codecov[bot] commented 7 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (1c733a1) 78.09% compared to head (097e004) 78.11%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #1192 +/- ## ========================================== + Coverage 78.09% 78.11% +0.01% ========================================== Files 53 53 Lines 7077 7077 ========================================== + Hits 5527 5528 +1 + Misses 1550 1549 -1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

kiplingw commented 7 months ago

Excellent work @tyler92!