From 60959f2b491876199879d97c8ed956eabb0c2e73 Mon Sep 17 00:00:00 2001 From: Sebastian Pipping Date: Fri, 14 May 2021 20:09:22 +0200 Subject: [PATCH 1/2] lib: Fix accounting of CDATA sections inside of general entities --- expat/Changes | 9 +++++---- expat/lib/xmlparse.c | 20 ++++++++++++-------- 2 files changed, 17 insertions(+), 12 deletions(-) diff --git a/expat/Changes b/expat/Changes index ebd238bb..60912e51 100644 --- a/expat/Changes +++ b/expat/Changes @@ -4,7 +4,7 @@ NOTE: We are looking for help with a few things: Release X.X.X XXX XXXXX XX XXXX Security fixes: - #34 #466 CVE-2013-0340/CWE-776 -- Protect against billion laughs attacks + #34 #466 #484 CVE-2013-0340/CWE-776 -- Protect against billion laughs attacks (denial-of-service; flavors targeting CPU time or RAM or both, leveraging general entities or parameter entities or both) by tracking and limiting the input amplification factor @@ -21,18 +21,18 @@ Release X.X.X XXX XXXXX XX XXXX for UTF-16 payloads containing CDATA sections. New features: - #34 #466 Add two new API functions to further tighten billion laughs + #34 #466 #484 Add two new API functions to further tighten billion laughs protection parameters when desired. - XML_SetBillionLaughsAttackProtectionMaximumAmplification - XML_SetBillionLaughsAttackProtectionActivationThreshold Please see file "doc/reference.html" for more details. If you ever need to increase the defaults for non-attack XML payload, please file a bug report with libexpat. - #34 #466 Introduce environment switches EXPAT_ACCOUNTING_DEBUG=(0|1|2|3) + #34 #466 #484 Introduce environment switches EXPAT_ACCOUNTING_DEBUG=(0|1|2|3) and EXPAT_ENTITY_DEBUG=(0|1) for runtime debugging of accounting and entity processing; specific behavior of these values may change in the future. - #34 #466 xmlwf: Add arguments "-a FACTOR" and "-b BYTES" to further tighten + #34 #466 #484 xmlwf: Add arguments "-a FACTOR" and "-b BYTES" to further tighten billion laughs protection parameters when desired. If you ever need to increase the defaults for non-attack XML payload, please file a bug report with libexpat. @@ -51,6 +51,7 @@ Release X.X.X XXX XXXXX XX XXXX and Clang LeakSan JetBrains + OSS-Fuzz Release 2.3.0 Thu March 25 2021 Bug fixes: diff --git a/expat/lib/xmlparse.c b/expat/lib/xmlparse.c index a1aadd86..6a59342c 100644 --- a/expat/lib/xmlparse.c +++ b/expat/lib/xmlparse.c @@ -462,7 +462,8 @@ static enum XML_Error doContent(XML_Parser parser, int startTagLevel, XML_Bool haveMore, enum XML_Account account); static enum XML_Error doCdataSection(XML_Parser parser, const ENCODING *, const char **startPtr, const char *end, - const char **nextPtr, XML_Bool haveMore); + const char **nextPtr, XML_Bool haveMore, + enum XML_Account account); #ifdef XML_DTD static enum XML_Error doIgnoreSection(XML_Parser parser, const ENCODING *, const char **startPtr, const char *end, @@ -3096,7 +3097,8 @@ doContent(XML_Parser parser, int startTagLevel, const ENCODING *enc, /* END disabled code */ else if (parser->m_defaultHandler) reportDefault(parser, enc, s, next); - result = doCdataSection(parser, enc, &next, end, nextPtr, haveMore); + result + = doCdataSection(parser, enc, &next, end, nextPtr, haveMore, account); if (result != XML_ERROR_NONE) return result; else if (! next) { @@ -3725,9 +3727,9 @@ addBinding(XML_Parser parser, PREFIX *prefix, const ATTRIBUTE_ID *attId, static enum XML_Error PTRCALL cdataSectionProcessor(XML_Parser parser, const char *start, const char *end, const char **endPtr) { - enum XML_Error result - = doCdataSection(parser, parser->m_encoding, &start, end, endPtr, - (XML_Bool)! parser->m_parsingStatus.finalBuffer); + enum XML_Error result = doCdataSection( + parser, parser->m_encoding, &start, end, endPtr, + (XML_Bool)! parser->m_parsingStatus.finalBuffer, XML_ACCOUNT_DIRECT); if (result != XML_ERROR_NONE) return result; if (start) { @@ -3747,7 +3749,8 @@ cdataSectionProcessor(XML_Parser parser, const char *start, const char *end, */ static enum XML_Error doCdataSection(XML_Parser parser, const ENCODING *enc, const char **startPtr, - const char *end, const char **nextPtr, XML_Bool haveMore) { + const char *end, const char **nextPtr, XML_Bool haveMore, + enum XML_Account account) { const char *s = *startPtr; const char **eventPP; const char **eventEndPP; @@ -3766,11 +3769,12 @@ doCdataSection(XML_Parser parser, const ENCODING *enc, const char **startPtr, const char *next = s; /* in case of XML_TOK_NONE or XML_TOK_PARTIAL */ int tok = XmlCdataSectionTok(enc, s, end, &next); #ifdef XML_DTD - if (! accountingDiffTolerated(parser, tok, s, next, __LINE__, - XML_ACCOUNT_DIRECT)) { + if (! accountingDiffTolerated(parser, tok, s, next, __LINE__, account)) { accountingOnAbort(parser); return XML_ERROR_AMPLIFICATION_LIMIT_BREACH; } +#else + UNUSED_P(account); #endif *eventEndPP = next; switch (tok) { From 77cfb8f4cd9679cef27ae9bc38e39ac51235af2d Mon Sep 17 00:00:00 2001 From: Sebastian Pipping Date: Fri, 14 May 2021 20:26:26 +0200 Subject: [PATCH 2/2] tests: Cover accounting of CDATA sections inside of general entities --- expat/tests/runtests.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/expat/tests/runtests.c b/expat/tests/runtests.c index 0e2b49fa..e3944561 100644 --- a/expat/tests/runtests.c +++ b/expat/tests/runtests.c @@ -11318,6 +11318,16 @@ START_TEST(test_accounting_precision) { /* CDATA */ {"", NULL, NULL, 0, filled_later}, + /* The following is the essence of this OSS-Fuzz finding: + https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=34302 + https://oss-fuzz.com/testcase-detail/4860575394955264 + */ + {"333\">\n" + "]>\n" + "&e;\n", + NULL, NULL, sizeof(XML_Char) * strlen("111333"), + filled_later}, /* Conditional sections */ {"