|
| 1 | +From 89a9c6807c982b4fa8aa806dd72771d6642dd8a1 Mon Sep 17 00:00:00 2001 |
| 2 | +From: Berkay Eren Ürün <berkay.ueruen@siemens.com |
| 3 | +Date: Wed, 19 Mar 2025 02:20:49 +0100 |
| 4 | +Subject: [PATCH] Stop updating m_eventPtr on exit for reentry |
| 5 | + |
| 6 | +The fix for recursive entity processing introduced a reenter flag that |
| 7 | +returns the execution from the current function and switches to entity |
| 8 | +processing. |
| 9 | + |
| 10 | +The same fix also updates the m_eventPtr during this switch. However |
| 11 | +this update changes the behaviour in certain cases as the older version |
| 12 | +does not update the m_eventPtr while recursing into entity processing. |
| 13 | + |
| 14 | +This commit removes the pointer update and restores the old behaviour. |
| 15 | + |
| 16 | +Upstream Patch Reference: https://patch-diff.githubusercontent.com/raw/libexpat/libexpat/pull/989.patch |
| 17 | +--- |
| 18 | + Changes | 15 ++++++++++++ |
| 19 | + lib/xmlparse.c | 12 ++++++--- |
| 20 | + tests/common.c | 25 +++++++++++++++++++ |
| 21 | + tests/common.h | 2 ++ |
| 22 | + tests/misc_tests.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++ |
| 23 | + 5 files changed, 112 insertions(+), 3 deletions(-) |
| 24 | + |
| 25 | +diff --git a/Changes b/Changes |
| 26 | +index 75c62d6..8c4ed04 100644 |
| 27 | +--- a/Changes |
| 28 | ++++ b/Changes |
| 29 | +@@ -29,6 +29,21 @@ |
| 30 | + !! THANK YOU! Sebastian Pipping -- Berlin, 2024-03-09 !! |
| 31 | + !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! |
| 32 | + |
| 33 | ++ Bug fixes: |
| 34 | ++ #980 #989 Restore event pointer behavior from Expat 2.6.4 |
| 35 | ++ (that the fix to CVE-2024-8176 changed in 2.7.0); |
| 36 | ++ affected API functions are: |
| 37 | ++ - XML_GetCurrentByteCount |
| 38 | ++ - XML_GetCurrentByteIndex |
| 39 | ++ - XML_GetCurrentColumnNumber |
| 40 | ++ - XML_GetCurrentLineNumber |
| 41 | ++ - XML_GetInputContext |
| 42 | ++ |
| 43 | ++ Special thanks to: |
| 44 | ++ Berkay Eren Ürün |
| 45 | ++ and |
| 46 | ++ Perl XML::Parser |
| 47 | ++ |
| 48 | + Security fixes: |
| 49 | + #893 #??? CVE-2024-8176 -- Fix crash from chaining a large number |
| 50 | + of entities caused by stack overflow by resolving use of |
| 51 | +diff --git a/lib/xmlparse.c b/lib/xmlparse.c |
| 52 | +index 0bf913c..9ec287c 100644 |
| 53 | +--- a/lib/xmlparse.c |
| 54 | ++++ b/lib/xmlparse.c |
| 55 | +@@ -3754,12 +3754,13 @@ doContent(XML_Parser parser, int startTagLevel, const ENCODING *enc, |
| 56 | + break; |
| 57 | + /* LCOV_EXCL_STOP */ |
| 58 | + } |
| 59 | +- *eventPP = s = next; |
| 60 | + switch (parser->m_parsingStatus.parsing) { |
| 61 | + case XML_SUSPENDED: |
| 62 | ++ *eventPP = next; |
| 63 | + *nextPtr = next; |
| 64 | + return XML_ERROR_NONE; |
| 65 | + case XML_FINISHED: |
| 66 | ++ *eventPP = next; |
| 67 | + return XML_ERROR_ABORTED; |
| 68 | + case XML_PARSING: |
| 69 | + if (parser->m_reenter) { |
| 70 | +@@ -3768,6 +3769,7 @@ doContent(XML_Parser parser, int startTagLevel, const ENCODING *enc, |
| 71 | + } |
| 72 | + /* Fall through */ |
| 73 | + default:; |
| 74 | ++ *eventPP = s = next; |
| 75 | + } |
| 76 | + } |
| 77 | + /* not reached */ |
| 78 | +@@ -4684,12 +4686,13 @@ doCdataSection(XML_Parser parser, const ENCODING *enc, const char **startPtr, |
| 79 | + /* LCOV_EXCL_STOP */ |
| 80 | + } |
| 81 | + |
| 82 | +- *eventPP = s = next; |
| 83 | + switch (parser->m_parsingStatus.parsing) { |
| 84 | + case XML_SUSPENDED: |
| 85 | ++ *eventPP = next; |
| 86 | + *nextPtr = next; |
| 87 | + return XML_ERROR_NONE; |
| 88 | + case XML_FINISHED: |
| 89 | ++ *eventPP = next; |
| 90 | + return XML_ERROR_ABORTED; |
| 91 | + case XML_PARSING: |
| 92 | + if (parser->m_reenter) { |
| 93 | +@@ -4697,6 +4700,7 @@ doCdataSection(XML_Parser parser, const ENCODING *enc, const char **startPtr, |
| 94 | + } |
| 95 | + /* Fall through */ |
| 96 | + default:; |
| 97 | ++ *eventPP = s = next; |
| 98 | + } |
| 99 | + } |
| 100 | + /* not reached */ |
| 101 | +@@ -6307,12 +6311,13 @@ epilogProcessor(XML_Parser parser, const char *s, const char *end, |
| 102 | + default: |
| 103 | + return XML_ERROR_JUNK_AFTER_DOC_ELEMENT; |
| 104 | + } |
| 105 | +- parser->m_eventPtr = s = next; |
| 106 | + switch (parser->m_parsingStatus.parsing) { |
| 107 | + case XML_SUSPENDED: |
| 108 | ++ parser->m_eventPtr = next; |
| 109 | + *nextPtr = next; |
| 110 | + return XML_ERROR_NONE; |
| 111 | + case XML_FINISHED: |
| 112 | ++ parser->m_eventPtr = next; |
| 113 | + return XML_ERROR_ABORTED; |
| 114 | + case XML_PARSING: |
| 115 | + if (parser->m_reenter) { |
| 116 | +@@ -6320,6 +6325,7 @@ epilogProcessor(XML_Parser parser, const char *s, const char *end, |
| 117 | + } |
| 118 | + /* Fall through */ |
| 119 | + default:; |
| 120 | ++ parser->m_eventPtr = s = next; |
| 121 | + } |
| 122 | + } |
| 123 | + } |
| 124 | +diff --git a/tests/common.c b/tests/common.c |
| 125 | +index 3aea8d7..b267dbb 100644 |
| 126 | +--- a/tests/common.c |
| 127 | ++++ b/tests/common.c |
| 128 | +@@ -42,6 +42,8 @@ |
| 129 | + */ |
| 130 | + |
| 131 | + #include <assert.h> |
| 132 | ++#include <errno.h> |
| 133 | ++#include <stdint.h> // for SIZE_MAX |
| 134 | + #include <stdio.h> |
| 135 | + #include <string.h> |
| 136 | + |
| 137 | +@@ -294,3 +296,26 @@ duff_reallocator(void *ptr, size_t size) { |
| 138 | + g_reallocation_count--; |
| 139 | + return realloc(ptr, size); |
| 140 | + } |
| 141 | ++ |
| 142 | ++// Portable remake of strndup(3) for C99; does not care about space efficiency |
| 143 | ++char * |
| 144 | ++portable_strndup(const char *s, size_t n) { |
| 145 | ++ if ((s == NULL) || (n == SIZE_MAX)) { |
| 146 | ++ errno = EINVAL; |
| 147 | ++ return NULL; |
| 148 | ++ } |
| 149 | ++ |
| 150 | ++ char *const buffer = (char *)malloc(n + 1); |
| 151 | ++ if (buffer == NULL) { |
| 152 | ++ errno = ENOMEM; |
| 153 | ++ return NULL; |
| 154 | ++ } |
| 155 | ++ |
| 156 | ++ errno = 0; |
| 157 | ++ |
| 158 | ++ memcpy(buffer, s, n); |
| 159 | ++ |
| 160 | ++ buffer[n] = '\0'; |
| 161 | ++ |
| 162 | ++ return buffer; |
| 163 | ++} |
| 164 | +diff --git a/tests/common.h b/tests/common.h |
| 165 | +index bc4c7da..8871130 100644 |
| 166 | +--- a/tests/common.h |
| 167 | ++++ b/tests/common.h |
| 168 | +@@ -146,6 +146,8 @@ extern void *duff_allocator(size_t size); |
| 169 | + |
| 170 | + extern void *duff_reallocator(void *ptr, size_t size); |
| 171 | + |
| 172 | ++extern char *portable_strndup(const char *s, size_t n); |
| 173 | ++ |
| 174 | + #endif /* XML_COMMON_H */ |
| 175 | + |
| 176 | + #ifdef __cplusplus |
| 177 | +diff --git a/tests/misc_tests.c b/tests/misc_tests.c |
| 178 | +index f9a78f6..2b9f793 100644 |
| 179 | +--- a/tests/misc_tests.c |
| 180 | ++++ b/tests/misc_tests.c |
| 181 | +@@ -561,6 +561,66 @@ START_TEST(test_renter_loop_finite_content) { |
| 182 | + } |
| 183 | + END_TEST |
| 184 | + |
| 185 | ++// Inspired by function XML_OriginalString of Perl's XML::Parser |
| 186 | ++static char * |
| 187 | ++dup_original_string(XML_Parser parser) { |
| 188 | ++ const int byte_count = XML_GetCurrentByteCount(parser); |
| 189 | ++ |
| 190 | ++ assert_true(byte_count >= 0); |
| 191 | ++ |
| 192 | ++ int offset = -1; |
| 193 | ++ int size = -1; |
| 194 | ++ |
| 195 | ++ const char *const context = XML_GetInputContext(parser, &offset, &size); |
| 196 | ++ |
| 197 | ++#if XML_CONTEXT_BYTES > 0 |
| 198 | ++ assert_true(context != NULL); |
| 199 | ++ assert_true(offset >= 0); |
| 200 | ++ assert_true(size >= 0); |
| 201 | ++ return portable_strndup(context + offset, byte_count); |
| 202 | ++#else |
| 203 | ++ assert_true(context == NULL); |
| 204 | ++ return NULL; |
| 205 | ++#endif |
| 206 | ++} |
| 207 | ++ |
| 208 | ++static void |
| 209 | ++on_characters_issue_980(void *userData, const XML_Char *s, int len) { |
| 210 | ++ (void)s; |
| 211 | ++ (void)len; |
| 212 | ++ XML_Parser parser = (XML_Parser)userData; |
| 213 | ++ |
| 214 | ++ char *const original_string = dup_original_string(parser); |
| 215 | ++ |
| 216 | ++#if XML_CONTEXT_BYTES > 0 |
| 217 | ++ assert_true(original_string != NULL); |
| 218 | ++ assert_true(strcmp(original_string, "&draft.day;") == 0); |
| 219 | ++ free(original_string); |
| 220 | ++#else |
| 221 | ++ assert_true(original_string == NULL); |
| 222 | ++#endif |
| 223 | ++} |
| 224 | ++ |
| 225 | ++START_TEST(test_misc_expected_event_ptr_issue_980) { |
| 226 | ++ // NOTE: This is a tiny subset of sample "REC-xml-19980210.xml" |
| 227 | ++ // from Perl's XML::Parser |
| 228 | ++ const char *const doc = "<!DOCTYPE day [\n" |
| 229 | ++ " <!ENTITY draft.day '10'>\n" |
| 230 | ++ "]>\n" |
| 231 | ++ "<day>&draft.day;</day>\n"; |
| 232 | ++ |
| 233 | ++ XML_Parser parser = XML_ParserCreate(NULL); |
| 234 | ++ XML_SetUserData(parser, parser); |
| 235 | ++ XML_SetCharacterDataHandler(parser, on_characters_issue_980); |
| 236 | ++ |
| 237 | ++ assert_true(_XML_Parse_SINGLE_BYTES(parser, doc, (int)strlen(doc), |
| 238 | ++ /*isFinal=*/XML_TRUE) |
| 239 | ++ == XML_STATUS_OK); |
| 240 | ++ |
| 241 | ++ XML_ParserFree(parser); |
| 242 | ++} |
| 243 | ++END_TEST |
| 244 | ++ |
| 245 | + void |
| 246 | + make_miscellaneous_test_case(Suite *s) { |
| 247 | + TCase *tc_misc = tcase_create("miscellaneous tests"); |
| 248 | +@@ -588,4 +648,5 @@ make_miscellaneous_test_case(Suite *s) { |
| 249 | + tcase_add_test(tc_misc, test_misc_resumeparser_not_crashing); |
| 250 | + tcase_add_test(tc_misc, test_misc_stopparser_rejects_unstarted_parser); |
| 251 | + tcase_add_test__if_xml_ge(tc_misc, test_renter_loop_finite_content); |
| 252 | ++ tcase_add_test(tc_misc, test_misc_expected_event_ptr_issue_980); |
| 253 | + } |
| 254 | +-- |
| 255 | +2.45.4 |
| 256 | + |
0 commit comments