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

[tyler92]

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]

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

poco/Foundation/src/TextIterator.cpp

Lines 111 to 115 in cefab15

	while (-1 > n && (_end - it) >= -n - read)
	{
	while (read < -n && it != _end)
	{
	*p++ = *it++;

[obiltschnig]

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

[micheleselea]

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]

@micheleselea Did you compile with address sanitizer enabled?

[micheleselea]

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

[tyler92]

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

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

[obiltschnig]

I also cannot reproduce this on macOS with ASAN enabled. I've also added assertions to catch the buffer overflow. Maybe this was fixed with #4320?

[obiltschnig]

Anyway, I have added an assertion to guard against buffer overflows caused by buggy TextEncoding implementations.

[micheleselea]

I tested with the assert you put in the code and in my version of poco I don't have the problem. I tested with the crash-7e3fdbcc15ad941711a3a1d2502ac293a272c267.txt proposed and with testUTF32Invalid1 and testUTF32Invalid2
I checked and I have the fix #4320 in my version
Tested with windows 11 and Visual studio 2022

[tyler92]

@obiltschnig You are right, after #4320 the issue is not reproduced (and reproduced before that patch). I used the "devel" branch which seems a little bit outdated. Thanks, we can close this issue now.