Out of bound read in std::strtol while parsing HTTP requests

[tyler92]

The std::strtol requires the input string to be a zero-terminated string, but the HTTP parsing procedure works with a binary buffer and there is no guarantee that the last byte is zero. It may lead to out-of-bound read, which is an undefined behavior and might cause it to crash. The list of affected functions:

http.cc:291          State ResponseLineStep::apply(StreamCursor& cursor)
http.cc:460          BodyStep::Chunk::Result BodyStep::Chunk::parse(StreamCursor& cursor)
http_header.cc:229   void CacheControl::parseRaw(const char* str, size_t len)

Example of sanitizer report used in fuzzing test:

==2060939==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x50c0008718c0 at pc 0x55892bade8af bp 0x7ffeeea689b0 sp 0x7ffeeea68170
READ of size 4 at 0x50c0008718c0 thread T0
    #0 0x55892bade8ae in StrtolFixAndCheck(void*, char const*, char**, char*, int) asan_interceptors.cpp.o
    #1 0x55892baca271 in strtol (pistache/build/tests/fuzzers/fuzz_parser+0x1d8271) (BuildId: b2cb0c20f13ffb84cf16359224e2ad7c4dcc6bc8)
    #2 0x55892bb6f65f in Pistache::Http::Private::BodyStep::Chunk::parse(Pistache::StreamCursor&) pistache/src/common/http.cc:460:35
    #3 0x55892bb6e85f in Pistache::Http::Private::BodyStep::parseTransferEncoding(Pistache::StreamCursor&, std::shared_ptr<Pistache::Http::Header::TransferEncoding> const&) pistache/src/common/http.cc:507:44
    #4 0x55892bb6d8db in Pistache::Http::Private::BodyStep::apply(Pistache::StreamCursor&) pistache/src/common/http.cc:397:24
    #5 0x55892bb7075a in Pistache::Http::Private::ParserBase::parse() pistache/src/common/http.cc:545:36
    #6 0x55892bb378c3 in fuzz_request_parser(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&)::$_0::operator()() const pistache/tests/fuzzers/fuzz_parser.cpp:82:48
    #7 0x55892bb256c4 in void ignoreExceptions<fuzz_request_parser(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&)::$_0>(fuzz_request_parser(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&)::$_0 const&) pistache/tests/fuzzers/fuzz_parser.cpp:17:9
    #8 0x55892bb2538c in fuzz_request_parser(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&) pistache/tests/fuzzers/fuzz_parser.cpp:82:9
    #9 0x55892bb2711a in LLVMFuzzerTestOneInput pistache/tests/fuzzers/fuzz_parser.cpp:153:9
    #10 0x55892ba2c8f0 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) (pistache/build/tests/fuzzers/fuzz_parser+0x13a8f0) (BuildId: b2cb0c20f13ffb84cf16359224e2ad7c4dcc6bc8)
    #11 0x55892ba2c065 in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool, bool*) (pistache/build/tests/fuzzers/fuzz_parser+0x13a065) (BuildId: b2cb0c20f13ffb84cf16359224e2ad7c4dcc6bc8)
    #12 0x55892ba2d845 in fuzzer::Fuzzer::MutateAndTestOne() (pistache/build/tests/fuzzers/fuzz_parser+0x13b845) (BuildId: b2cb0c20f13ffb84cf16359224e2ad7c4dcc6bc8)
    #13 0x55892ba2e455 in fuzzer::Fuzzer::Loop(std::vector<fuzzer::SizedFile, std::allocator<fuzzer::SizedFile>>&) (pistache/build/tests/fuzzers/fuzz_parser+0x13c455) (BuildId: b2cb0c20f13ffb84cf16359224e2ad7c4dcc6bc8)
    #14 0x55892ba1c5bb in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) (pistache/build/tests/fuzzers/fuzz_parser+0x12a5bb) (BuildId: b2cb0c20f13ffb84cf16359224e2ad7c4dcc6bc8)
    #15 0x55892ba455f2 in main (pistache/build/tests/fuzzers/fuzz_parser+0x1535f2) (BuildId: b2cb0c20f13ffb84cf16359224e2ad7c4dcc6bc8)
    #16 0x7fef53e0ed8f in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
    #17 0x7fef53e0ee3f in __libc_start_main csu/../csu/libc-start.c:392:3
    #18 0x55892ba11a24 in _start (pistache/build/tests/fuzzers/fuzz_parser+0x11fa24) (BuildId: b2cb0c20f13ffb84cf16359224e2ad7c4dcc6bc8)

0x50c0008718c0 is located 0 bytes after 128-byte region [0x50c000871840,0x50c0008718c0)
allocated by thread T0 here:
    #0 0x55892bb1e32d in operator new(unsigned long) (pistache/build/tests/fuzzers/fuzz_parser+0x22c32d) (BuildId: b2cb0c20f13ffb84cf16359224e2ad7c4dcc6bc8)
    #1 0x55892bba8a88 in std::__new_allocator<char>::allocate(unsigned long, void const*) /usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/new_allocator.h:137:27
    #2 0x55892bba893a in std::allocator_traits<std::allocator<char>>::allocate(std::allocator<char>&, unsigned long) /usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/alloc_traits.h:464:20
    #3 0x55892bba809b in std::_Vector_base<char, std::allocator<char>>::_M_allocate(unsigned long) /usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/stl_vector.h:378:20
    #4 0x55892bba73dc in void std::vector<char, std::allocator<char>>::_M_realloc_insert<char const&>(__gnu_cxx::__normal_iterator<char*, std::vector<char, std::allocator<char>>>, char const&) /usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/vector.tcc:453:33
    #5 0x55892bba7027 in std::vector<char, std::allocator<char>>::push_back(char const&) /usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/stl_vector.h:1287:4
    #6 0x55892bba6d1b in std::back_insert_iterator<std::vector<char, std::allocator<char>>>::operator=(char const&) /usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/stl_iterator.h:735:13
    #7 0x55892bba6b3e in std::back_insert_iterator<std::vector<char, std::allocator<char>>> std::__copy_move<false, false, std::random_access_iterator_tag>::__copy_m<char const*, std::back_insert_iterator<std::vector<char, std::allocator<char>>>>(char const*, char const*, std::back_insert_iterator<std::vector<char, std::allocator<char>>>) /usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/stl_algobase.h:385:18
    #8 0x55892bba68a5 in std::back_insert_iterator<std::vector<char, std::allocator<char>>> std::__copy_move_a2<false, char const*, std::back_insert_iterator<std::vector<char, std::allocator<char>>>>(char const*, char const*, std::back_insert_iterator<std::vector<char, std::allocator<char>>>) /usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/stl_algobase.h:494:14
    #9 0x55892bba63f5 in std::back_insert_iterator<std::vector<char, std::allocator<char>>> std::__copy_move_a1<false, char const*, std::back_insert_iterator<std::vector<char, std::allocator<char>>>>(char const*, char const*, std::back_insert_iterator<std::vector<char, std::allocator<char>>>) /usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/stl_algobase.h:522:14
    #10 0x55892bba5eaa in std::back_insert_iterator<std::vector<char, std::allocator<char>>> std::__copy_move_a<false, char const*, std::back_insert_iterator<std::vector<char, std::allocator<char>>>>(char const*, char const*, std::back_insert_iterator<std::vector<char, std::allocator<char>>>) /usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/stl_algobase.h:530:3
    #11 0x55892bba59a3 in std::back_insert_iterator<std::vector<char, std::allocator<char>>> std::copy<char const*, std::back_insert_iterator<std::vector<char, std::allocator<char>>>>(char const*, char const*, std::back_insert_iterator<std::vector<char, std::allocator<char>>>) /usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/stl_algobase.h:619:14
    #12 0x55892bb70cb2 in Pistache::ArrayStreamBuf<char>::feed(char const*, unsigned long) pistache/src/../include/pistache/stream.h:111:13
    #13 0x55892bb709aa in Pistache::Http::Private::ParserBase::feed(char const*, unsigned long) pistache/src/common/http.cc:558:27
    #14 0x55892bb252ad in fuzz_request_parser(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&) pistache/tests/fuzzers/fuzz_parser.cpp:79:17
    #15 0x55892bb2711a in LLVMFuzzerTestOneInput pistache/tests/fuzzers/fuzz_parser.cpp:153:9
    #16 0x55892ba2c8f0 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) (pistache/build/tests/fuzzers/fuzz_parser+0x13a8f0) (BuildId: b2cb0c20f13ffb84cf16359224e2ad7c4dcc6bc8)
    #17 0x55892ba2c065 in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool, bool*) (pistache/build/tests/fuzzers/fuzz_parser+0x13a065) (BuildId: b2cb0c20f13ffb84cf16359224e2ad7c4dcc6bc8)
    #18 0x55892ba2d845 in fuzzer::Fuzzer::MutateAndTestOne() (pistache/build/tests/fuzzers/fuzz_parser+0x13b845) (BuildId: b2cb0c20f13ffb84cf16359224e2ad7c4dcc6bc8)
    #19 0x55892ba2e455 in fuzzer::Fuzzer::Loop(std::vector<fuzzer::SizedFile, std::allocator<fuzzer::SizedFile>>&) (pistache/build/tests/fuzzers/fuzz_parser+0x13c455) (BuildId: b2cb0c20f13ffb84cf16359224e2ad7c4dcc6bc8)
    #20 0x55892ba1c5bb in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) (pistache/build/tests/fuzzers/fuzz_parser+0x12a5bb) (BuildId: b2cb0c20f13ffb84cf16359224e2ad7c4dcc6bc8)
    #21 0x55892ba455f2 in main (pistache/build/tests/fuzzers/fuzz_parser+0x1535f2) (BuildId: b2cb0c20f13ffb84cf16359224e2ad7c4dcc6bc8)
    #22 0x7fef53e0ed8f in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16

SUMMARY: AddressSanitizer: heap-buffer-overflow asan_interceptors.cpp.o in StrtolFixAndCheck(void*, char const*, char**, char*, int)
Shadow bytes around the buggy address:
  0x50c000871600: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fa
  0x50c000871680: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd
  0x50c000871700: fd fd fd fd fd fd fd fd fa fa fa fa fa fa fa fa
  0x50c000871780: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fa
  0x50c000871800: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
=>0x50c000871880: 00 00 00 00 00 00 00 00[fa]fa fa fa fa fa fa fa
  0x50c000871900: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x50c000871980: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x50c000871a00: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x50c000871a80: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fa
  0x50c000871b00: 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
==2060939==ABORTING

[kiplingw]

Another good find @tyler92. If or when you're ready with a PR just let us know.

[alexprabhat99]

@kiplingw Can I take an attempt at this?

[kiplingw]

Yes, certainly @alexprabhat99 and thanks for showing initiative. @dgreatwood might have some thoughts on this too.

[alexprabhat99]

@kiplingw Could you please check out my PR #1201 which attempts to fix this issue? Your feedback would certainly help me improve. Thanks in advance!