cheng/libexpat
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge pull request #318 from libexpat/issue-317

Browse Source 
Deny internal entities closing the doctype (for #317)
This commit is contained in:
Sebastian Pipping 2019-09-03 22:00:41 +02:00 committed by GitHub
commit 6da1f19625
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 86 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
expat/Changes
View File

@ -3,6 +3,11 @@ NOTE: We are looking for help with a few things:
If you can help, please get in touch. Thanks!
Release x.x.x xxx xxx xx xxxx
Security fixes:
#317 #318 Fix heap overflow triggered by XML_GetCurrentLineNumber
(or XML_GetCurrentColumnNumber), and deny internal entities
closing the doctype
Bug fixes:
#240 Fix cases where XML_StopParser did not have any effect
when called from inside of an end element handler
@ -81,6 +86,7 @@ Release x.x.x xxx xxx xx xxxx
Special thanks to:
David Loffredo
Joonun Jang
Khajapasha Mohammed
Kishore Kunche
Marco Maggi

20
expat/lib/xmlparse.c
View File

@ -401,7 +401,7 @@ static enum XML_Error initializeEncoding(XML_Parser parser);
static enum XML_Error doProlog(XML_Parser parser, const ENCODING *enc,
const char *s, const char *end, int tok,
const char *next, const char **nextPtr,
XML_Bool haveMore);
XML_Bool haveMore, XML_Bool allowClosingDoctype);
static enum XML_Error processInternalEntity(XML_Parser parser, ENTITY *entity,
XML_Bool betweenDecl);
static enum XML_Error doContent(XML_Parser parser, int startTagLevel,
@ -4046,7 +4046,7 @@ externalParEntProcessor(XML_Parser parser, const char *s, const char *end,
parser->m_processor = prologProcessor;
return doProlog(parser, parser->m_encoding, s, end, tok, next, nextPtr,
(XML_Bool)! parser->m_parsingStatus.finalBuffer);
(XML_Bool)! parser->m_parsingStatus.finalBuffer, XML_TRUE);
}
static enum XML_Error PTRCALL
@ -4090,12 +4090,13 @@ prologProcessor(XML_Parser parser, const char *s, const char *end,
const char *next = s;
int tok = XmlPrologTok(parser->m_encoding, s, end, &next);
return doProlog(parser, parser->m_encoding, s, end, tok, next, nextPtr,
(XML_Bool)! parser->m_parsingStatus.finalBuffer);
(XML_Bool)! parser->m_parsingStatus.finalBuffer, XML_TRUE);
}
static enum XML_Error
doProlog(XML_Parser parser, const ENCODING *enc, const char *s, const char *end,
int tok, const char *next, const char **nextPtr, XML_Bool haveMore) {
int tok, const char *next, const char **nextPtr, XML_Bool haveMore,
XML_Bool allowClosingDoctype) {
#ifdef XML_DTD
static const XML_Char externalSubsetName[] = {ASCII_HASH, '\0'};
#endif /* XML_DTD */
@ -4271,6 +4272,11 @@ doProlog(XML_Parser parser, const ENCODING *enc, const char *s, const char *end,
}
break;
case XML_ROLE_DOCTYPE_CLOSE:
if (allowClosingDoctype != XML_TRUE) {
/* Must not close doctype from within expanded parameter entities */
return XML_ERROR_INVALID_TOKEN;
}
if (parser->m_doctypeName) {
parser->m_startDoctypeDeclHandler(
parser->m_handlerArg, parser->m_doctypeName, parser->m_doctypeSysid,
@ -5174,7 +5180,7 @@ processInternalEntity(XML_Parser parser, ENTITY *entity, XML_Bool betweenDecl) {
int tok
= XmlPrologTok(parser->m_internalEncoding, textStart, textEnd, &next);
result = doProlog(parser, parser->m_internalEncoding, textStart, textEnd,
tok, next, &next, XML_FALSE);
tok, next, &next, XML_FALSE, XML_FALSE);
} else
#endif /* XML_DTD */
result = doContent(parser, parser->m_tagLevel, parser->m_internalEncoding,
@ -5217,7 +5223,7 @@ internalEntityProcessor(XML_Parser parser, const char *s, const char *end,
int tok
= XmlPrologTok(parser->m_internalEncoding, textStart, textEnd, &next);
result = doProlog(parser, parser->m_internalEncoding, textStart, textEnd,
tok, next, &next, XML_FALSE);
tok, next, &next, XML_FALSE, XML_TRUE);
} else
#endif /* XML_DTD */
result = doContent(parser, openEntity->startTagLevel,
@ -5244,7 +5250,7 @@ internalEntityProcessor(XML_Parser parser, const char *s, const char *end,
parser->m_processor = prologProcessor;
tok = XmlPrologTok(parser->m_encoding, s, end, &next);
return doProlog(parser, parser->m_encoding, s, end, tok, next, nextPtr,
(XML_Bool)! parser->m_parsingStatus.finalBuffer);
(XML_Bool)! parser->m_parsingStatus.finalBuffer, XML_TRUE);
} else
#endif /* XML_DTD */
{

67
expat/tests/runtests.c
View File

@ -7480,6 +7480,69 @@ START_TEST(test_misc_stop_during_end_handler_issue_240_2) {
}
END_TEST
#ifdef XML_DTD
START_TEST(test_misc_deny_internal_entity_closing_doctype_issue_317) {
const char *const inputOne = "<!DOCTYPE d [\n"
"<!ENTITY % e ']><d/>'>\n"
"\n"
"%e;";
const char *const inputTwo = "<!DOCTYPE d [\n"
"<!ENTITY % e1 ']><d/>'><!ENTITY % e2 '&e1;'>\n"
"\n"
"%e2;";
const char *const inputThree = "<!DOCTYPE d [\n"
"<!ENTITY % e ']><d'>\n"
"\n"
"%e;";
const char *const inputIssue317 = "<!DOCTYPE doc [\n"
"<!ENTITY % foo ']>\n"
"<doc>Hell<oc (#PCDATA)*>'>\n"
"%foo;\n"
"]>\n"
"<doc>Hello, world</dVc>";
const char *const inputs[] = {inputOne, inputTwo, inputThree, inputIssue317};
size_t inputIndex = 0;
for (; inputIndex < sizeof(inputs) / sizeof(inputs[0]); inputIndex++) {
XML_Parser parser;
enum XML_Status parseResult;
int setParamEntityResult;
XML_Size lineNumber;
XML_Size columnNumber;
const char *const input = inputs[inputIndex];
parser = XML_ParserCreate(NULL);
setParamEntityResult
= XML_SetParamEntityParsing(parser, XML_PARAM_ENTITY_PARSING_ALWAYS);
if (setParamEntityResult != 1)
fail("Failed to set XML_PARAM_ENTITY_PARSING_ALWAYS.");
parseResult = XML_Parse(parser, input, (int)strlen(input), 0);
if (parseResult != XML_STATUS_ERROR) {
parseResult = XML_Parse(parser, "", 0, 1);
if (parseResult != XML_STATUS_ERROR) {
fail("Parsing was expected to fail but succeeded.");
}
}
if (XML_GetErrorCode(parser) != XML_ERROR_INVALID_TOKEN)
fail("Error code does not match XML_ERROR_INVALID_TOKEN");
lineNumber = XML_GetCurrentLineNumber(parser);
if (lineNumber != 4)
fail("XML_GetCurrentLineNumber does not work as expected.");
columnNumber = XML_GetCurrentColumnNumber(parser);
if (columnNumber != 0)
fail("XML_GetCurrentColumnNumber does not work as expected.");
XML_ParserFree(parser);
}
}
END_TEST
#endif
static void
alloc_setup(void) {
XML_Memory_Handling_Suite memsuite = {duff_allocator, duff_reallocator, free};
@ -11408,6 +11471,10 @@ make_suite(void) {
tcase_add_test(tc_misc, test_misc_utf16le);
tcase_add_test(tc_misc, test_misc_stop_during_end_handler_issue_240_1);
tcase_add_test(tc_misc, test_misc_stop_during_end_handler_issue_240_2);
#ifdef XML_DTD
tcase_add_test(tc_misc,
test_misc_deny_internal_entity_closing_doctype_issue_317);
#endif
suite_add_tcase(s, tc_alloc);
tcase_add_checked_fixture(tc_alloc, alloc_setup, alloc_teardown);