diff options
author | Sebastian Pipping <sebastian@pipping.org> | 2024-03-07 22:14:09 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2024-03-07 22:14:09 +0100 |
commit | 5026213864ba1a11ef03ba2e8111af8654e9404d (patch) | |
tree | 732476f710f29f18b8c8d2ebe53c8cdc022c8c9a | |
parent | 27525adabdac2a48a5ce5d2f5588fce741a0c8e3 (diff) | |
parent | 072eca0b72373da103ce15f8f62d1d7b52695454 (diff) | |
download | expat-5026213864ba1a11ef03ba2e8111af8654e9404d.tar.gz |
Merge pull request #842 from libexpat/issue-839-billion-laughs-isolated-external-parser
Prevent billion laughs attacks in isolated external parser (part of #839)
-rw-r--r-- | expat/lib/xmlparse.c | 6 | ||||
-rw-r--r-- | expat/tests/acc_tests.c | 59 |
2 files changed, 64 insertions, 1 deletions
diff --git a/expat/lib/xmlparse.c b/expat/lib/xmlparse.c index 8e667fcb..b9f50aba 100644 --- a/expat/lib/xmlparse.c +++ b/expat/lib/xmlparse.c @@ -7787,6 +7787,8 @@ copyString(const XML_Char *s, const XML_Memory_Handling_Suite *memsuite) { static float accountingGetCurrentAmplification(XML_Parser rootParser) { + // 1.........1.........12 => 22 + const size_t lenOfShortestInclude = sizeof("<!ENTITY a SYSTEM 'b'>") - 1; const XmlBigCount countBytesOutput = rootParser->m_accounting.countBytesDirect + rootParser->m_accounting.countBytesIndirect; @@ -7794,7 +7796,9 @@ accountingGetCurrentAmplification(XML_Parser rootParser) { = rootParser->m_accounting.countBytesDirect ? (countBytesOutput / (float)(rootParser->m_accounting.countBytesDirect)) - : 1.0f; + : ((lenOfShortestInclude + + rootParser->m_accounting.countBytesIndirect) + / (float)lenOfShortestInclude); assert(! rootParser->m_parentParser); return amplificationFactor; } diff --git a/expat/tests/acc_tests.c b/expat/tests/acc_tests.c index e1c4b7f7..f193aa58 100644 --- a/expat/tests/acc_tests.c +++ b/expat/tests/acc_tests.c @@ -378,6 +378,63 @@ START_TEST(test_helper_unsigned_char_to_printable) { fail("unsignedCharToPrintable result mistaken"); } END_TEST + +START_TEST(test_amplification_isolated_external_parser) { + // NOTE: Length 44 is precisely twice the length of "<!ENTITY a SYSTEM 'b'>" + // (22) that is used in function accountingGetCurrentAmplification in + // xmlparse.c. + // 1.........1.........1.........1.........1..4 => 44 + const char doc[] = "<!ENTITY % p1 '123456789_123456789_1234567'>"; + const int docLen = (int)sizeof(doc) - 1; + const float maximumToleratedAmplification = 2.0f; + + struct TestCase { + int offsetOfThreshold; + enum XML_Status expectedStatus; + }; + + struct TestCase cases[] = { + {-2, XML_STATUS_ERROR}, {-1, XML_STATUS_ERROR}, {0, XML_STATUS_ERROR}, + {+1, XML_STATUS_OK}, {+2, XML_STATUS_OK}, + }; + + for (size_t i = 0; i < sizeof(cases) / sizeof(cases[0]); i++) { + const int offsetOfThreshold = cases[i].offsetOfThreshold; + const enum XML_Status expectedStatus = cases[i].expectedStatus; + const unsigned long long activationThresholdBytes + = docLen + offsetOfThreshold; + + set_subtest("offsetOfThreshold=%d, expectedStatus=%d", offsetOfThreshold, + expectedStatus); + + XML_Parser parser = XML_ParserCreate(NULL); + assert_true(parser != NULL); + + assert_true(XML_SetBillionLaughsAttackProtectionMaximumAmplification( + parser, maximumToleratedAmplification) + == XML_TRUE); + assert_true(XML_SetBillionLaughsAttackProtectionActivationThreshold( + parser, activationThresholdBytes) + == XML_TRUE); + + XML_Parser ext_parser = XML_ExternalEntityParserCreate(parser, NULL, NULL); + assert_true(ext_parser != NULL); + + const enum XML_Status actualStatus + = _XML_Parse_SINGLE_BYTES(ext_parser, doc, docLen, XML_TRUE); + + assert_true(actualStatus == expectedStatus); + if (actualStatus != XML_STATUS_OK) { + assert_true(XML_GetErrorCode(ext_parser) + == XML_ERROR_AMPLIFICATION_LIMIT_BREACH); + } + + XML_ParserFree(ext_parser); + XML_ParserFree(parser); + } +} +END_TEST + #endif // XML_GE == 1 void @@ -390,6 +447,8 @@ make_accounting_test_case(Suite *s) { tcase_add_test(tc_accounting, test_accounting_precision); tcase_add_test(tc_accounting, test_billion_laughs_attack_protection_api); tcase_add_test(tc_accounting, test_helper_unsigned_char_to_printable); + tcase_add_test__ifdef_xml_dtd(tc_accounting, + test_amplification_isolated_external_parser); #else UNUSED_P(s); #endif /* XML_GE == 1 */ |