commit 05d87eb116ddde35bfa4e4c1d2ec7bcbda38c09b Author: Tomas Korbar Date: Wed Sep 11 13:48:58 2024 +0200 Fix CVE-2024-45490 https://github.com/libexpat/libexpat/pull/890 diff --git a/expat/doc/reference.html b/expat/doc/reference.html index a10f3cb..d618bd8 100644 --- a/expat/doc/reference.html +++ b/expat/doc/reference.html @@ -1098,7 +1098,9 @@ containing part (or perhaps all) of the document. The number of bytes of s that are part of the document is indicated by len. This means that s doesn't have to be null terminated. It also means that if len is larger than the number of bytes in the block of -memory that s points at, then a memory fault is likely. The +memory that s points at, then a memory fault is likely. +Negative values for len are rejected since Expat 2.2.1. +The isFinal parameter informs the parser that this is the last piece of the document. Frequently, the last piece is empty (i.e. len is zero.) @@ -1114,11 +1116,17 @@ XML_ParseBuffer(XML_Parser p, int isFinal);
+

This is just like XML_Parse, except in this case Expat provides the buffer. By obtaining the buffer from Expat with the XML_GetBuffer function, the application can avoid double copying of the input. +

+ +

+Negative values for len are rejected since Expat 2.6.3. +

XML_GetBuffer

diff --git a/expat/lib/xmlparse.c b/expat/lib/xmlparse.c index 0896b16..f54e258 100644 --- a/expat/lib/xmlparse.c +++ b/expat/lib/xmlparse.c @@ -1998,6 +1998,12 @@ XML_ParseBuffer(XML_Parser parser, int len, int isFinal) { if (parser == NULL) return XML_STATUS_ERROR; + + if (len < 0) { + parser->m_errorCode = XML_ERROR_INVALID_ARGUMENT; + return XML_STATUS_ERROR; + } + switch (parser->m_parsingStatus.parsing) { case XML_SUSPENDED: parser->m_errorCode = XML_ERROR_SUSPENDED; diff --git a/expat/tests/runtests.c b/expat/tests/runtests.c index 93adc45..ed88f9f 100644 --- a/expat/tests/runtests.c +++ b/expat/tests/runtests.c @@ -3856,6 +3856,57 @@ START_TEST(test_empty_parse) { } END_TEST +/* Test XML_Parse for len < 0 */ +START_TEST(test_negative_len_parse) { + const char *const doc = ""; + for (int isFinal = 0; isFinal < 2; isFinal++) { + XML_Parser parser = XML_ParserCreate(NULL); + + if (XML_GetErrorCode(parser) != XML_ERROR_NONE) + fail("There was not supposed to be any initial parse error."); + + const enum XML_Status status = XML_Parse(parser, doc, -1, isFinal); + + if (status != XML_STATUS_ERROR) + fail("Negative len was expected to fail the parse but did not."); + + if (XML_GetErrorCode(parser) != XML_ERROR_INVALID_ARGUMENT) + fail("Parse error does not match XML_ERROR_INVALID_ARGUMENT."); + + XML_ParserFree(parser); + } +} +END_TEST + +/* Test XML_ParseBuffer for len < 0 */ +START_TEST(test_negative_len_parse_buffer) { + const char *const doc = ""; + for (int isFinal = 0; isFinal < 2; isFinal++) { + XML_Parser parser = XML_ParserCreate(NULL); + + if (XML_GetErrorCode(parser) != XML_ERROR_NONE) + fail("There was not supposed to be any initial parse error."); + + void *const buffer = XML_GetBuffer(parser, (int)strlen(doc)); + + if (buffer == NULL) + fail("XML_GetBuffer failed."); + + memcpy(buffer, doc, strlen(doc)); + + const enum XML_Status status = XML_ParseBuffer(parser, -1, isFinal); + + if (status != XML_STATUS_ERROR) + fail("Negative len was expected to fail the parse but did not."); + + if (XML_GetErrorCode(parser) != XML_ERROR_INVALID_ARGUMENT) + fail("Parse error does not match XML_ERROR_INVALID_ARGUMENT."); + + XML_ParserFree(parser); + } +} +END_TEST + /* Test odd corners of the XML_GetBuffer interface */ static enum XML_Status get_feature(enum XML_FeatureEnum feature_id, long *presult) { @@ -12937,6 +12988,8 @@ make_suite(void) { tcase_add_test__ifdef_xml_dtd(tc_basic, test_user_parameters); tcase_add_test__ifdef_xml_dtd(tc_basic, test_ext_entity_ref_parameter); tcase_add_test(tc_basic, test_empty_parse); + tcase_add_test(tc_basic, test_negative_len_parse); + tcase_add_test(tc_basic, test_negative_len_parse_buffer); tcase_add_test(tc_basic, test_get_buffer_1); tcase_add_test(tc_basic, test_get_buffer_2); #if defined(XML_CONTEXT_BYTES)