pocoproject / poco

The POCO C++ Libraries are powerful cross-platform C++ libraries for building network- and internet-based applications that run on desktop, server, mobile, IoT, and embedded systems.
https://pocoproject.org
Other
8.29k stars 2.14k forks source link

Net: stack-buffer-overflow if HTTP request contains a header with invalid UTF32 sequence #4690

Open tyler92 opened 1 week ago

tyler92 commented 1 week ago

Describe the bug

During a fuzzing test, ASAN reported a stack-buffer-overflow error in TextIterator::operator * (). It happened due to a missing check for a buffer size.

To Reproduce

Poco::MemoryInputStream stream(input.data(), input.size());
Poco::Net::HTTPRequest request;
request.read(stream);

with the following input: crash-7e3fdbcc15ad941711a3a1d2502ac293a272c267.txt

Expected behavior

ASAN doesn't report any errors.

Logs

=================================================================
==1706181==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7f453060a2e4 at pc 0x7f453302f52e bp 0x7ffc446dc010 sp 0x7ffc446dc008
WRITE of size 1 at 0x7f453060a2e4 thread T0
    #0 0x7f453302f52d in Poco::TextIterator::operator*() const poco/Foundation/src/TextIterator.cpp:115:9
    #1 0x7f4533026f6a in Poco::TextConverter::convert(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>&, int (*)(int)) poco/Foundation/src/TextConverter.cpp:53:11
    #2 0x7f4533026f6a in Poco::TextConverter::convert(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>&) poco/Foundation/src/TextConverter.cpp:118:9
    #3 0x7f453369fb7b in Poco::Net::MessageHeader::decodeRFC2047(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&) poco/Net/src/MessageHeader.cpp:352:14
    #4 0x7f453369aa08 in Poco::Net::MessageHeader::decodeWord(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&) poco/Net/src/MessageHeader.cpp:414:3
    #5 0x7f45336996f7 in Poco::Net::MessageHeader::read(std::istream&) poco/Net/src/MessageHeader.cpp:108:13
    #6 0x7f453361cd2e in Poco::Net::HTTPRequest::read(std::istream&) poco/Net/src/HTTPRequest.cpp:257:15
    #7 0x6408958aab77 in LLVMFuzzerTestOneInput poco/fuzzing/http_messages_fuzzer.cc:30:13
    #8 0x6408957b6aa0 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) (poco/fuzzing/prefix/bin/http_messages_fuzzer+0x4daa0)
    #9 0x6408957a0bf2 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) (poco/fuzzing/prefix/bin/http_messages_fuzzer+0x37bf2)
    #10 0x6408957a6636 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) (poco/fuzzing/prefix/bin/http_messages_fuzzer+0x3d636)
    #11 0x6408957cf7a2 in main (poco/fuzzing/prefix/bin/http_messages_fuzzer+0x667a2)
    #12 0x7f4532829d8f in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
    #13 0x7f4532829e3f in __libc_start_main csu/../csu/libc-start.c:392:3
    #14 0x64089579bbd4 in _start (poco/fuzzing/prefix/bin/http_messages_fuzzer+0x32bd4)

Address 0x7f453060a2e4 is located in stack of thread T0 at offset 36 in frame
    #0 0x7f453302ef6f in Poco::TextIterator::operator*() const poco/Foundation/src/TextIterator.cpp:95

  This frame has 1 object(s):
    [32, 36) 'buffer' (line 100) <== Memory access at offset 36 overflows this variable
HINT: this may be a false positive if your program uses some custom stack unwind mechanism, swapcontext or vfork
      (longjmp and C++ exceptions *are* supported)
SUMMARY: AddressSanitizer: stack-buffer-overflow poco/Foundation/src/TextIterator.cpp:115:9 in Poco::TextIterator::operator*() const
Shadow bytes around the buggy address:
  0x7f453060a000: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5
  0x7f453060a080: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5
  0x7f453060a100: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5
  0x7f453060a180: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5
  0x7f453060a200: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5
=>0x7f453060a280: f5 f5 f5 f5 f5 f5 f5 f5 f1 f1 f1 f1[04]f3 f3 f3
  0x7f453060a300: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x7f453060a380: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x7f453060a400: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x7f453060a480: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x7f453060a500: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
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
==1706181==ABORTING

I prepared two unit tests that are failing now for simplification:

void TextIteratorTest::testUTF32Invalid1()
{
  UTF32Encoding encoding;
  const Poco::UInt32 data[] = {0x00000041, 0xffffffef, 0x00000041, 0x00000041, 0x00000041, 0x00000041, 0x00};
  std::string text((const char*) data, 24);
  TextIterator it(text, encoding);
  TextIterator end(text);

  assertTrue (it != end);
  assertTrue (*it++ == 0x41);
  assertTrue (it != end);
  assertTrue (*it++ == -1);
}

void TextIteratorTest::testUTF32Invalid2()
{
  UTF32Encoding encoding;
  const Poco::UInt32 data[] = {0x00000041, 0xfffffffe, 0xfffffffe, 0x00};
  std::string text((const char*) data, 12);
  TextIterator it(text, encoding);
  TextIterator end(text);

  assertTrue (it != end);
  assertTrue (*it++ == 0x41);
  assertTrue (it != end);
  assertTrue (*it++ == -1);
}
tyler92 commented 1 week ago

The reason is that for line 115 there is no check that p++ is not out of bounds of buffer.

https://github.com/pocoproject/poco/blob/cefab15f9f7d39d8c61fde7f60b788e70a11e5a8/Foundation/src/TextIterator.cpp#L111-L115

obiltschnig commented 1 week ago

That damn UTF32Encoding class is really a source of constant amusement. See also: #4320

micheleselea commented 5 days ago

I tried in VisualStudio 2022 Poco::MemoryInputStream stream(input.data(), input.size()); Poco::Net::HTTPRequest request; request.read(stream); with crash-7e3fdbcc15ad941711a3a1d2502ac293a272c267.txt proposed but I can't get the error, can be only a problem in Linux environment?

tyler92 commented 5 days ago

@micheleselea Did you compile with address sanitizer enabled?

micheleselea commented 3 days ago

I did the test in debug environment so I suppose it's enabled by default, but I double check it

tyler92 commented 9 hours ago

so I suppose it's enabled by default, but I double check it

No, it's not, you need to enable it explicitly