diff --git a/cdoc/XmlReader.cpp b/cdoc/XmlReader.cpp index 60cc3b6d..1fc8a382 100644 --- a/cdoc/XmlReader.cpp +++ b/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(context); @@ -96,13 +80,24 @@ 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 XMLReader::readBase64() { if (!d) return {}; xmlTextReaderRead(d); + if (xmlTextReaderNodeType(d) == XML_READER_TYPE_ENTITY_REFERENCE) + return {}; return libcdoc::Crypto::decodeBase64(xmlTextReaderConstValue(d)); } @@ -110,5 +105,7 @@ std::string XMLReader::readText() { if (!d) return {}; xmlTextReaderRead(d); + if (xmlTextReaderNodeType(d) == XML_READER_TYPE_ENTITY_REFERENCE) + return {}; return tostring(xmlTextReaderConstValue(d)); } diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 31f758d0..95ac912e 100644 --- a/test/CMakeLists.txt +++ b/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 ) diff --git a/test/libcdoc_boost.cpp b/test/libcdoc_boost.cpp index 3ed35c89..bf8ecd32 100644 --- a/test/libcdoc_boost.cpp +++ b/test/libcdoc_boost.cpp @@ -27,8 +27,11 @@ #include #include #include +#include #include +#include + #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 data; + libcdoc::VectorSource source; +}; + const map 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("text"); + 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(""); +} + +BOOST_AUTO_TEST_CASE(RejectsInternalEntityInAttribute) +{ + rejectsUnsupportedXml("]>"); +} + +BOOST_AUTO_TEST_CASE(RejectsInternalEntityInText) +{ + rejectsUnsupportedXml("]>&xxe;"); +} + +BOOST_AUTO_TEST_CASE(RejectsExternalEntityInAttribute) +{ + rejectsUnsupportedXml("]>"); +} + +BOOST_AUTO_TEST_CASE(RejectsExternalEntityInText) +{ + rejectsUnsupportedXml("]>&xxe;"); +} + +BOOST_AUTO_TEST_CASE(DoesNotChangeGlobalExternalEntityLoader) +{ + auto loader = xmlGetExternalEntityLoader(); + XMLSource input(""); + 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.