From 51c7019069b862e88d94ed228659e70bddd5de09 Mon Sep 17 00:00:00 2001
From: Sebastian Pipping <sebastian@pipping.org>
Date: Mon, 21 Oct 2024 01:42:54 +0200
Subject: [PATCH 1/3] lib: Make XML_StopParser refuse to stop/suspend an
 unstarted parser

---
 expat/lib/expat.h    | 4 +++-
 expat/lib/xmlparse.c | 6 ++++++
 2 files changed, 9 insertions(+), 1 deletion(-)

Index: expat-2.5.0/lib/expat.h
===================================================================
--- expat-2.5.0.orig/lib/expat.h
+++ expat-2.5.0/lib/expat.h
@@ -127,7 +127,9 @@ enum XML_Error {
   /* Added in 2.3.0. */
   XML_ERROR_NO_BUFFER,
   /* Added in 2.4.0. */
-  XML_ERROR_AMPLIFICATION_LIMIT_BREACH
+  XML_ERROR_AMPLIFICATION_LIMIT_BREACH,
+  /* Added in 2.6.4. */
+  XML_ERROR_NOT_STARTED,
 };
 
 enum XML_Content_Type {
Index: expat-2.5.0/lib/xmlparse.c
===================================================================
--- expat-2.5.0.orig/lib/xmlparse.c
+++ expat-2.5.0/lib/xmlparse.c
@@ -2171,6 +2171,9 @@ XML_StopParser(XML_Parser parser, XML_Bo
   if (parser == NULL)
     return XML_STATUS_ERROR;
   switch (parser->m_parsingStatus.parsing) {
+  case XML_INITIALIZED:
+    parser->m_errorCode = XML_ERROR_NOT_STARTED;
+    return XML_STATUS_ERROR;
   case XML_SUSPENDED:
     if (resumable) {
       parser->m_errorCode = XML_ERROR_SUSPENDED;
@@ -2181,7 +2184,7 @@ XML_StopParser(XML_Parser parser, XML_Bo
   case XML_FINISHED:
     parser->m_errorCode = XML_ERROR_FINISHED;
     return XML_STATUS_ERROR;
-  default:
+  case XML_PARSING:
     if (resumable) {
 #ifdef XML_DTD
       if (parser->m_isParamEntity) {
@@ -2192,6 +2195,9 @@ XML_StopParser(XML_Parser parser, XML_Bo
       parser->m_parsingStatus.parsing = XML_SUSPENDED;
     } else
       parser->m_parsingStatus.parsing = XML_FINISHED;
+    break;
+  default:
+    assert(0);
   }
   return XML_STATUS_OK;
 }
@@ -2456,6 +2462,9 @@ XML_ErrorString(enum XML_Error code) {
   case XML_ERROR_AMPLIFICATION_LIMIT_BREACH:
     return XML_L(
         "limit on input amplification factor (from DTD and entities) breached");
+  /* Added in 2.6.4. */
+  case XML_ERROR_NOT_STARTED:
+    return XML_L("parser not started");
   }
   return NULL;
 }
Index: expat-2.5.0/tests/runtests.c
===================================================================
--- expat-2.5.0.orig/tests/runtests.c
+++ expat-2.5.0/tests/runtests.c
@@ -7991,6 +7991,28 @@ START_TEST(test_misc_tag_mismatch_reset_
 }
 END_TEST
 
+START_TEST(test_misc_resumeparser_not_crashing) {
+  XML_Parser parser = XML_ParserCreate(NULL);
+  XML_GetBuffer(parser, 1);
+  XML_StopParser(parser, /*resumable=*/XML_TRUE);
+  XML_ResumeParser(parser); // could crash here, previously
+  XML_ParserFree(parser);
+}
+END_TEST
+
+START_TEST(test_misc_stopparser_rejects_unstarted_parser) {
+  const XML_Bool cases[] = {XML_TRUE, XML_FALSE};
+  for (size_t i = 0; i < sizeof(cases) / sizeof(cases[0]); i++) {
+    const XML_Bool resumable = cases[i];
+    XML_Parser parser = XML_ParserCreate(NULL);
+    assert_true(XML_GetErrorCode(parser) == XML_ERROR_NONE);
+    assert_true(XML_StopParser(parser, resumable) == XML_STATUS_ERROR);
+    assert_true(XML_GetErrorCode(parser) == XML_ERROR_NOT_STARTED);
+    XML_ParserFree(parser);
+  }
+}
+END_TEST
+
 static void
 alloc_setup(void) {
   XML_Memory_Handling_Suite memsuite = {duff_allocator, duff_reallocator, free};
@@ -12447,6 +12469,8 @@ make_suite(void) {
   tcase_add_test__ifdef_xml_dtd(
       tc_misc, test_misc_deny_internal_entity_closing_doctype_issue_317);
   tcase_add_test(tc_misc, test_misc_tag_mismatch_reset_leak);
+  tcase_add_test(tc_misc, test_misc_resumeparser_not_crashing);
+  tcase_add_test(tc_misc, test_misc_stopparser_rejects_unstarted_parser);
 
   suite_add_tcase(s, tc_alloc);
   tcase_add_checked_fixture(tc_alloc, alloc_setup, alloc_teardown);