|
| 1 | +From 4a32da87e931ba54393d465bb77c40b5c33d343b Mon Sep 17 00:00:00 2001 |
| 2 | +From: Rhodri James < [email protected]> |
| 3 | +Date: Wed, 17 Aug 2022 18:26:18 +0100 |
| 4 | +Subject: [PATCH] Ensure raw tagnames are safe exiting internalEntityParser |
| 5 | + |
| 6 | +It is possible to concoct a situation in which parsing is |
| 7 | +suspended while substituting in an internal entity, so that |
| 8 | +XML_ResumeParser directly uses internalEntityProcessor as |
| 9 | +its processor. If the subsequent parse includes some unclosed |
| 10 | +tags, this will return without calling storeRawNames to ensure |
| 11 | +that the raw versions of the tag names are stored in memory other |
| 12 | +than the parse buffer itself. If the parse buffer is then changed |
| 13 | +or reallocated (for example if processing a file line by line), |
| 14 | +badness will ensue. |
| 15 | + |
| 16 | +This patch ensures storeRawNames is always called when needed |
| 17 | +after calling doContent. The earlier call do doContent does |
| 18 | +not need the same protection; it only deals with entity |
| 19 | +substitution, which cannot leave unbalanced tags, and in any |
| 20 | +case the raw names will be pointing into the stored entity |
| 21 | +value not the parse buffer. |
| 22 | +--- |
| 23 | + expat/lib/xmlparse.c | 13 +++++++++---- |
| 24 | + 1 file changed, 9 insertions(+), 4 deletions(-) |
| 25 | + |
| 26 | +diff --git a/expat/lib/xmlparse.c b/expat/lib/xmlparse.c |
| 27 | +index 7bcabf7f4..d73f419cf 100644 |
| 28 | +--- a/expat/lib/xmlparse.c |
| 29 | ++++ b/expat/lib/xmlparse.c |
| 30 | +@@ -5826,10 +5826,15 @@ internalEntityProcessor(XML_Parser parser, const char *s, const char *end, |
| 31 | + { |
| 32 | + parser->m_processor = contentProcessor; |
| 33 | + /* see externalEntityContentProcessor vs contentProcessor */ |
| 34 | +- return doContent(parser, parser->m_parentParser ? 1 : 0, parser->m_encoding, |
| 35 | +- s, end, nextPtr, |
| 36 | +- (XML_Bool)! parser->m_parsingStatus.finalBuffer, |
| 37 | +- XML_ACCOUNT_DIRECT); |
| 38 | ++ result = doContent(parser, parser->m_parentParser ? 1 : 0, |
| 39 | ++ parser->m_encoding, s, end, nextPtr, |
| 40 | ++ (XML_Bool)! parser->m_parsingStatus.finalBuffer, |
| 41 | ++ XML_ACCOUNT_DIRECT); |
| 42 | ++ if (result == XML_ERROR_NONE) { |
| 43 | ++ if (! storeRawNames(parser)) |
| 44 | ++ return XML_ERROR_NO_MEMORY; |
| 45 | ++ } |
| 46 | ++ return result; |
| 47 | + } |
| 48 | + } |
| 49 | + |
| 50 | +From a7ce80a013f2a08cb1ac4aac368f2250eea03ebf Mon Sep 17 00:00:00 2001 |
| 51 | +From: Sebastian Pipping < [email protected]> |
| 52 | +Date: Sun, 11 Sep 2022 19:34:33 +0200 |
| 53 | +Subject: [PATCH 1/2] tests: Cover heap use-after-free issue in doContent |
| 54 | + |
| 55 | +--- |
| 56 | + expat/tests/runtests.c | 74 ++++++++++++++++++++++++++++++++++++++++++ |
| 57 | + 1 file changed, 74 insertions(+) |
| 58 | + |
| 59 | +diff --git a/expat/tests/runtests.c b/expat/tests/runtests.c |
| 60 | +index ea371b42f..ab3aff65b 100644 |
| 61 | +--- a/expat/tests/runtests.c |
| 62 | ++++ b/expat/tests/runtests.c |
| 63 | +@@ -4990,6 +4990,78 @@ START_TEST(test_suspend_resume_internal_entity) { |
| 64 | + } |
| 65 | + END_TEST |
| 66 | + |
| 67 | ++void |
| 68 | ++suspending_comment_handler(void *userData, const XML_Char *data) { |
| 69 | ++ UNUSED_P(data); |
| 70 | ++ XML_Parser parser = (XML_Parser)userData; |
| 71 | ++ XML_StopParser(parser, XML_TRUE); |
| 72 | ++} |
| 73 | ++ |
| 74 | ++START_TEST(test_suspend_resume_internal_entity_issue_629) { |
| 75 | ++ const char *const text |
| 76 | ++ = "<!DOCTYPE a [<!ENTITY e '<!--COMMENT-->a'>]><a>&e;<b>\n" |
| 77 | ++ "<" |
| 78 | ++ "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" |
| 79 | ++ "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" |
| 80 | ++ "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" |
| 81 | ++ "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" |
| 82 | ++ "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" |
| 83 | ++ "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" |
| 84 | ++ "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" |
| 85 | ++ "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" |
| 86 | ++ "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" |
| 87 | ++ "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" |
| 88 | ++ "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" |
| 89 | ++ "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" |
| 90 | ++ "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" |
| 91 | ++ "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" |
| 92 | ++ "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" |
| 93 | ++ "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" |
| 94 | ++ "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" |
| 95 | ++ "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" |
| 96 | ++ "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" |
| 97 | ++ "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" |
| 98 | ++ "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" |
| 99 | ++ "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" |
| 100 | ++ "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" |
| 101 | ++ "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" |
| 102 | ++ "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" |
| 103 | ++ "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" |
| 104 | ++ "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" |
| 105 | ++ "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" |
| 106 | ++ "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" |
| 107 | ++ "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" |
| 108 | ++ "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" |
| 109 | ++ "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" |
| 110 | ++ "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" |
| 111 | ++ "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" |
| 112 | ++ "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" |
| 113 | ++ "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" |
| 114 | ++ "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" |
| 115 | ++ "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" |
| 116 | ++ "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" |
| 117 | ++ "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" |
| 118 | ++ "/>" |
| 119 | ++ "</b></a>"; |
| 120 | ++ const size_t firstChunkSizeBytes = 54; |
| 121 | ++ |
| 122 | ++ XML_Parser parser = XML_ParserCreate(NULL); |
| 123 | ++ XML_SetUserData(parser, parser); |
| 124 | ++ XML_SetCommentHandler(parser, suspending_comment_handler); |
| 125 | ++ |
| 126 | ++ if (XML_Parse(parser, text, (int)firstChunkSizeBytes, XML_FALSE) |
| 127 | ++ != XML_STATUS_SUSPENDED) |
| 128 | ++ xml_failure(parser); |
| 129 | ++ if (XML_ResumeParser(parser) != XML_STATUS_OK) |
| 130 | ++ xml_failure(parser); |
| 131 | ++ if (XML_Parse(parser, text + firstChunkSizeBytes, |
| 132 | ++ (int)(strlen(text) - firstChunkSizeBytes), XML_TRUE) |
| 133 | ++ != XML_STATUS_OK) |
| 134 | ++ xml_failure(parser); |
| 135 | ++ XML_ParserFree(parser); |
| 136 | ++} |
| 137 | ++END_TEST |
| 138 | ++ |
| 139 | + /* Test syntax error is caught at parse resumption */ |
| 140 | + START_TEST(test_resume_entity_with_syntax_error) { |
| 141 | + const char *text = "<!DOCTYPE doc [\n" |
| 142 | +@@ -12016,6 +12088,8 @@ make_suite(void) { |
| 143 | + tcase_add_test(tc_basic, test_partial_char_in_epilog); |
| 144 | + tcase_add_test(tc_basic, test_hash_collision); |
| 145 | + tcase_add_test__ifdef_xml_dtd(tc_basic, test_suspend_resume_internal_entity); |
| 146 | ++ tcase_add_test__ifdef_xml_dtd(tc_basic, |
| 147 | ++ test_suspend_resume_internal_entity_issue_629); |
| 148 | + tcase_add_test__ifdef_xml_dtd(tc_basic, test_resume_entity_with_syntax_error); |
| 149 | + tcase_add_test__ifdef_xml_dtd(tc_basic, test_suspend_resume_parameter_entity); |
| 150 | + tcase_add_test(tc_basic, test_restart_on_error); |
| 151 | + |
| 152 | +diff -Naur a/expat/Changes b/expat/Changes |
| 153 | +--- a/expat/Changes 2022-03-28 21:11:43.000000000 +0000 |
| 154 | ++++ b/expat/Changes 2022-09-20 01:08:23.484300828 +0000 |
| 155 | +@@ -3,6 +3,11 @@ |
| 156 | + If you can help, please get in touch. Thanks! |
| 157 | + |
| 158 | + Release 2.4.8 Mon March 28 2022 |
| 159 | ++ Security fixes: |
| 160 | ++ #629 #640 CVE-2022-40674 -- Heap use-after-free vulnerability in |
| 161 | ++ function doContent. Expected impact is denial of service |
| 162 | ++ or potentially arbitrary code execution. |
| 163 | ++ |
| 164 | + Other changes: |
| 165 | + #587 pkg-config: Move "-lm" to section "Libs.private" |
| 166 | + #587 CMake|MSVC: Fix pkg-config section "Libs" |
| 167 | +@@ -20,6 +25,10 @@ |
| 168 | + evpobr |
| 169 | + Kai Pastor |
| 170 | + Sam James |
| 171 | ++ Felix Wilhelm |
| 172 | ++ Rhodri James |
| 173 | ++ and |
| 174 | ++ Google Project Zero |
| 175 | + |
| 176 | + Release 2.4.7 Fri March 4 2022 |
| 177 | + Bug fixes: |
0 commit comments