Do not change libxml2 global state

cdoc/XmlReader.cpp

@@ -43,22 +43,6 @@ static std::string tostring(pcxmlChar tmp)
     return result;
 }
 
-#if LIBXML_VERSION < 21300
-static xmlParserInputPtr
-nullExternalEntityLoader(const char *, const char *, xmlParserCtxtPtr)
-{
-    return nullptr;
-}
-
-struct XmlInit {
-    XmlInit() {
-        xmlSetExternalEntityLoader(nullExternalEntityLoader);
-        xmlSubstituteEntitiesDefault(0);
-    }
-};
-static XmlInit xmlInit;
-#endif
-
 XMLReader::XMLReader(libcdoc::DataSource &src)
     : d(xmlReaderForIO([](void *context, char *buffer, int len) -> int {
         auto *src = reinterpret_cast<DataSource *>(context);
@@ -96,19 +80,32 @@ bool XMLReader::isElement(const char *elem) const
 bool XMLReader::read()
 {
     if (!d) return false;
-    return xmlTextReaderRead(d) == 1;
+    if (xmlTextReaderRead(d) != 1)
+        return false;
+    switch(xmlTextReaderNodeType(d))
+    {
+    case XML_READER_TYPE_DOCUMENT_TYPE:
+    case XML_READER_TYPE_ENTITY_REFERENCE:
+        return false;
+    default:
+        return true;
+    }
 }
 
 std::vector<uint8_t> XMLReader::readBase64()
 {
     if (!d) return {};
     xmlTextReaderRead(d);
+    if (xmlTextReaderNodeType(d) == XML_READER_TYPE_ENTITY_REFERENCE)
+        return {};
     return libcdoc::Crypto::decodeBase64(xmlTextReaderConstValue(d));
 }
 
 std::string XMLReader::readText()
 {
     if (!d) return {};
     xmlTextReaderRead(d);
+    if (xmlTextReaderNodeType(d) == XML_READER_TYPE_ENTITY_REFERENCE)
+        return {};
     return tostring(xmlTextReaderConstValue(d));
 }

test/CMakeLists.txt

@@ -2,10 +2,12 @@ add_executable(unittests
     libcdoc_boost.cpp
     ../cdoc/Crypto.cpp
     ../cdoc/Tar.cpp
+    ../cdoc/XmlReader.cpp
 )
 
 target_link_libraries(unittests
     OpenSSL::SSL
+    LibXml2::LibXml2
     cdoc
     Boost::unit_test_framework
 )

test/libcdoc_boost.cpp

@@ -27,8 +27,11 @@
 #include <Recipient.h>
 #include <Tar.h>
 #include <Utils.h>
+#include <XmlReader.h>
 #include <cdoc/Crypto.h>
 
+#include <libxml/parser.h>
+
 #include "pipe.h"
 
 #ifndef DATA_DIR
@@ -73,6 +76,16 @@ constexpr string_view AESKey = "E165475C6D8B9DD0B696EE2A37D7176DFDF4D7B510406648
 
 constexpr string_view CDOC2HEADER = "CDOC\x02"sv;
 
+struct XMLSource {
+    explicit XMLSource(std::string_view xml)
+        : data(xml.cbegin(), xml.cend())
+        , source(data)
+    {}
+
+    std::vector<uint8_t> data;
+    libcdoc::VectorSource source;
+};
+
 const map<string, string> ExpectedParsedLabel {
     {"v", "1"},
     {"type", "ID-card"},
@@ -1135,6 +1148,66 @@ BOOST_AUTO_TEST_CASE(TruncatesOverlongNames)
 
 BOOST_AUTO_TEST_SUITE_END()
 
+BOOST_AUTO_TEST_SUITE(XMLReaderEntityHandling)
+
+BOOST_AUTO_TEST_CASE(ReadsPlainXml)
+{
+    XMLSource input("<root a=\"value\"><child>text</child></root>");
+    libcdoc::XMLReader reader(input.source);
+
+    BOOST_REQUIRE(reader.read());
+    BOOST_TEST(reader.isElement("root"));
+    BOOST_TEST(reader.attribute("a") == "value");
+
+    BOOST_REQUIRE(reader.read());
+    BOOST_TEST(reader.isElement("child"));
+    BOOST_TEST(reader.readText() == "text");
+}
+
+static void rejectsUnsupportedXml(std::string_view xml)
+{
+    XMLSource input(xml);
+    libcdoc::XMLReader reader(input.source);
+    BOOST_CHECK(!reader.read());
+}
+
+BOOST_AUTO_TEST_CASE(RejectsDoctype)
+{
+    rejectsUnsupportedXml("<!DOCTYPE root><root/>");
+}
+
+BOOST_AUTO_TEST_CASE(RejectsInternalEntityInAttribute)
+{
+    rejectsUnsupportedXml("<!DOCTYPE root [<!ENTITY xxe 'SECRET'>]><root a='&xxe;'/>");
+}
+
+BOOST_AUTO_TEST_CASE(RejectsInternalEntityInText)
+{
+    rejectsUnsupportedXml("<!DOCTYPE root [<!ENTITY xxe 'SECRET'>]><root>&xxe;</root>");
+}
+
+BOOST_AUTO_TEST_CASE(RejectsExternalEntityInAttribute)
+{
+    rejectsUnsupportedXml("<!DOCTYPE root [<!ENTITY xxe SYSTEM 'file:///etc/passwd'>]><root a='&xxe;'/>");
+}
+
+BOOST_AUTO_TEST_CASE(RejectsExternalEntityInText)
+{
+    rejectsUnsupportedXml("<!DOCTYPE root [<!ENTITY xxe SYSTEM 'file:///etc/passwd'>]><root>&xxe;</root>");
+}
+
+BOOST_AUTO_TEST_CASE(DoesNotChangeGlobalExternalEntityLoader)
+{
+    auto loader = xmlGetExternalEntityLoader();
+    XMLSource input("<root/>");
+    libcdoc::XMLReader reader(input.source);
+
+    BOOST_REQUIRE(reader.read());
+    BOOST_TEST(xmlGetExternalEntityLoader() == loader);
+}
+
+BOOST_AUTO_TEST_SUITE_END()
+
 // Coverage for libcdoc::Cleanser, the RAII guard used by CDoc2Reader::getFMK
 // and CDoc2Writer::buildHeader to wipe short-lived KEK / FMK material on
 // every exit including exceptions.