Sign Up
Log In
Log In
or
Sign Up
Places
All Projects
Status Monitor
Collapse sidebar
SUSE:SLE-12-SP4:GA
expat.35528
expat-CVE-2023-52425-1.patch
Overview
Repositories
Revisions
Requests
Users
Attributes
Meta
File expat-CVE-2023-52425-1.patch of Package expat.35528
From 119ae277abaabd4d17b2e64300fec712ef403b28 Mon Sep 17 00:00:00 2001 From: Snild Dolkow <snild@sony.com> Date: Thu, 28 Sep 2023 18:26:19 +0200 Subject: [PATCH] Grow buffer based on current size Until now, the buffer size to grow to has been calculated based on the distance from the current parse position to the end of the buffer. This means that the size of any already-parsed data was not considered, leading to inconsistent buffer growth. There was also a special case in XML_Parse() when XML_CONTEXT_BYTES was zero, where the buffer size would be set to twice the incoming string length. This patch replaces this with an XML_GetBuffer() call. Growing the buffer based on its total size makes its growth consistent. The commit includes a test that checks that we can reach the max buffer size (usually INT_MAX/2 + 1) regardless of previously parsed content. GitHub CI couldn't allocate the full 1GiB with MinGW/wine32, though it works locally with the same compiler and wine version. As a workaround, the test tries to malloc 1GiB, and reduces `maxbuf` to 512MiB in case of failure. --- expat/lib/xmlparse.c | 33 ++++++++++++++-------------- expat/tests/basic_tests.c | 45 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 17 deletions(-) Index: expat-2.4.4/lib/xmlparse.c =================================================================== --- expat-2.4.4.orig/lib/xmlparse.c +++ expat-2.4.4/lib/xmlparse.c @@ -1636,24 +1636,22 @@ XML_Parse(XML_Parser parser, const char XmlUpdatePosition(encoding, positionPtr, end, &position); nLeftOver = s + len - end; if (nLeftOver) { - if (buffer == NULL || nLeftOver > bufferLim - buffer) { - /* avoid _signed_ integer overflow */ - char *temp = NULL; - const int bytesToAllocate = (int)((unsigned)len * 2U); - if (bytesToAllocate > 0) { - temp = (buffer == NULL - ? (char *)MALLOC(bytesToAllocate) - : (char *)REALLOC(buffer, bytesToAllocate)); - } - if (temp == NULL) { - errorCode = XML_ERROR_NO_MEMORY; - eventPtr = eventEndPtr = NULL; - processor = errorProcessor; - return XML_STATUS_ERROR; - } - buffer = temp; - bufferLim = buffer + bytesToAllocate; + // Back up and restore the parsing status to avoid XML_ERROR_SUSPENDED + // (and XML_ERROR_FINISHED) from XML_GetBuffer. + const enum XML_Parsing originalStatus = parser->m_parsingStatus.parsing; + parser->m_parsingStatus.parsing = XML_PARSING; + void *const temp = XML_GetBuffer(parser, nLeftOver); + parser->m_parsingStatus.parsing = originalStatus; + if (temp == NULL) { + // NOTE: parser->m_errorCode has already been set by XML_GetBuffer(). + parser->m_eventPtr = parser->m_eventEndPtr = NULL; + parser->m_processor = errorProcessor; + return XML_STATUS_ERROR; } + // Since we know that the buffer was empty and XML_CONTEXT_BYTES is 0, we + // don't have any data to preserve, and can copy straight into the start + // of the buffer rather than the GetBuffer return pointer (which may be + // pointing further into the allocated buffer). memcpy(buffer, end, nLeftOver); } bufferPtr = buffer; @@ -1784,7 +1782,7 @@ XML_GetBuffer(XML_Parser parser, int len } else { char *newBuf; - int bufferSize = (int)(bufferLim - bufferPtr); + int bufferSize = (int)(bufferLim - buffer); if (bufferSize == 0) bufferSize = INIT_BUFFER_SIZE; do { Index: expat-2.4.4/tests/runtests.c =================================================================== --- expat-2.4.4.orig/tests/runtests.c +++ expat-2.4.4/tests/runtests.c @@ -1803,6 +1803,51 @@ START_TEST(test_empty_element_abort) { } END_TEST +START_TEST(test_buffer_can_grow_to_max) { + const char *const prefixes[] = { + "", + "<", + "<x a='", + "<doc><x a='", + "<document><x a='", + "<averylongelementnamesuchthatitwillhopefullystretchacrossmultiplelinesand" + "lookprettyridiculousitsalsoveryhardtoreadandifyouredoingitihavetowonderif" + "youreallydonthaveanythingbettertodoofcourseiguessicouldveputsomethingbadin" + "herebutipromisethatididntheybtwhowgreatarespacesandpunctuationforhelping" + "withreadabilityprettygreatithinkanywaysthisisprobablylongenoughbye><x a='"}; + const int num_prefixes = sizeof(prefixes) / sizeof(prefixes[0]); + int maxbuf = INT_MAX / 2 + (INT_MAX & 1); // round up without overflow + if (sizeof(void *) < 8) { + // Looks like we have a 32-bit system. Can we make a big allocation? + void *big = malloc(maxbuf); + if (! big) { + // The big allocation failed. Let's be a little lenient. + maxbuf = maxbuf / 2; + } + free(big); + } + + for (int i = 0; i < num_prefixes; ++i) { + // set_subtest("\"%s\"", prefixes[i]); + XML_Parser parser = XML_ParserCreate(NULL); + const int prefix_len = (int)strlen(prefixes[i]); + const enum XML_Status s + = _XML_Parse_SINGLE_BYTES(parser, prefixes[i], prefix_len, XML_FALSE); + if (s != XML_STATUS_OK) + xml_failure(parser); + + // XML_CONTEXT_BYTES of the prefix may remain in the buffer; + // subtracting the whole prefix is easiest, and close enough. + assert_true(XML_GetBuffer(parser, maxbuf - prefix_len) != NULL); + // The limit should be consistent; no prefix should allow us to + // reach above the max buffer size. + assert_true(XML_GetBuffer(parser, maxbuf + 1) == NULL); + XML_ParserFree(parser); + } +} +END_TEST + + static Suite * make_suite(void) { @@ -1860,6 +1905,7 @@ make_suite(void) { tcase_add_test(tc_basic, test_misc_deny_internal_entity_closing_doctype_issue_317); #endif tcase_add_test(tc_basic, test_bad_doctype_utf8); + tcase_add_test(tc_basic, test_buffer_can_grow_to_max); suite_add_tcase(s, tc_namespace); tcase_add_checked_fixture(tc_namespace, Index: expat-2.4.4/tests/minicheck.h =================================================================== --- expat-2.4.4.orig/tests/minicheck.h +++ expat-2.4.4/tests/minicheck.h @@ -31,7 +31,13 @@ extern "C" { { #define END_TEST } } -#define fail(msg) _fail_unless(0, __FILE__, __LINE__, msg) +#define fail(msg) _fail(__FILE__, __LINE__, msg) +#define assert_true(cond) \ + do { \ + if (! (cond)) { \ + _fail(__FILE__, __LINE__, "check failed: " #cond); \ + } \ + } while (0) typedef void (*tcase_setup_function)(void); typedef void (*tcase_teardown_function)(void); @@ -73,6 +79,10 @@ void _check_set_test_info(char const *fu */ void _fail_unless(int condition, const char *file, int line, char *msg); +# if defined(__GNUC__) +__attribute__((noreturn)) +# endif +void _fail(const char *file, int line, const char *msg); Suite *suite_create(char *name); TCase *tcase_create(char *name); void suite_add_tcase(Suite *suite, TCase *tc); Index: expat-2.4.4/tests/minicheck.c =================================================================== --- expat-2.4.4.orig/tests/minicheck.c +++ expat-2.4.4/tests/minicheck.c @@ -167,6 +167,21 @@ _fail_unless(int condition, const char * longjmp(env, 1); } +void +_fail(const char *file, int line, const char *msg) { + /* Always print the error message so it isn't lost. In this case, + we have a failure, so there's no reason to be quiet about what + it is. + */ + _check_current_filename = file; + _check_current_lineno = line; + if (msg != NULL) { + const int has_newline = (msg[strlen(msg) - 1] == '\n'); + fprintf(stderr, "ERROR: %s%s", msg, has_newline ? "" : "\n"); + } + longjmp(env, 1); +} + int srunner_ntests_failed(SRunner *runner) {
Locations
Projects
Search
Status Monitor
Help
OpenBuildService.org
Documentation
API Documentation
Code of Conduct
Contact
Support
@OBShq
Terms
openSUSE Build Service is sponsored by
The Open Build Service is an
openSUSE project
.
Sign Up
Log In
Places
Places
All Projects
Status Monitor
