c/libxml2
1
0
Fork 0
mirror of https://gitlab.gnome.org/GNOME/libxml2 synced 2025-03-28 21:33:13 +00:00
Code Issues Packages Projects Releases Wiki Activity

Rework entity boundary checks

Browse Source 
Make sure to finish all entities in the internal subset. Nevertheless,
readd a sanity check in xmlParseStartTag2 that was lost in my previous
commit. Also add a sanity check in xmlPopInput. Popping an input
unexpectedly was the source of many recent memory bugs. The check
doesn't mitigate such issues but helps with diagnosis.

Always base entity boundary checks on the input ID, not the input
pointer. The pointer could have been reallocated to the old address.

Always throw a well-formedness error if a boundary check fails. In a
few places, a validity error was thrown.

Fix a few error codes and improve indentation.
This commit is contained in:
Nick Wellnhofer 2017-06-12 14:32:34 +02:00
parent 46dc989080
commit 5f440d8cad
9 changed files with 153 additions and 99 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

126
parser.c
View File

@ -2215,6 +2215,10 @@ xmlPopInput(xmlParserCtxtPtr ctxt) {
if (xmlParserDebugEntities)
xmlGenericError(xmlGenericErrorContext,
"Popping input %d\n", ctxt->inputNr);
if ((ctxt->inputNr > 1) && (ctxt->inSubset == 0) &&
(ctxt->instate != XML_PARSER_EOF))
xmlFatalErr(ctxt, XML_ERR_INTERNAL_ERROR,
"Unfinished entity outside the DTD");
xmlFreeInputStream(inputPop(ctxt));
if ((*ctxt->input->cur == 0) &&
(xmlParserInputGrow(ctxt->input, INPUT_CHUNK) <= 0))
@ -4837,7 +4841,8 @@ xmlParseCommentComplex(xmlParserCtxtPtr ctxt, xmlChar *buf,
} else {
if (inputid != ctxt->input->id) {
xmlFatalErrMsg(ctxt, XML_ERR_ENTITY_BOUNDARY,
"Comment doesn't start and stop in the same entity\n");
"Comment doesn't start and stop in the same"
" entity\n");
}
NEXT;
if ((ctxt->sax != NULL) && (ctxt->sax->comment != NULL) &&
@ -4985,7 +4990,8 @@ get_more:
if (in[2] == '>') {
if (ctxt->input->id != inputid) {
xmlFatalErrMsg(ctxt, XML_ERR_ENTITY_BOUNDARY,
"comment doesn't start and stop in the same entity\n");
"comment doesn't start and stop in the"
" same entity\n");
}
SKIP(3);
if ((ctxt->sax != NULL) && (ctxt->sax->comment != NULL) &&
@ -5153,7 +5159,7 @@ xmlParsePI(xmlParserCtxtPtr ctxt) {
int count = 0;
if ((RAW == '<') && (NXT(1) == '?')) {
xmlParserInputPtr input = ctxt->input;
int inputid = ctxt->input->id;
state = ctxt->instate;
ctxt->instate = XML_PARSER_PI;
/*
@ -5169,9 +5175,10 @@ xmlParsePI(xmlParserCtxtPtr ctxt) {
target = xmlParsePITarget(ctxt);
if (target != NULL) {
if ((RAW == '?') && (NXT(1) == '>')) {
if (input != ctxt->input) {
if (inputid != ctxt->input->id) {
xmlFatalErrMsg(ctxt, XML_ERR_ENTITY_BOUNDARY,
"PI declaration doesn't start and stop in the same entity\n");
"PI declaration doesn't start and stop in"
" the same entity\n");
}
SKIP(2);
@ -5253,9 +5260,10 @@ xmlParsePI(xmlParserCtxtPtr ctxt) {
xmlFatalErrMsgStr(ctxt, XML_ERR_PI_NOT_FINISHED,
"ParsePI: PI %s never end ...\n", target);
} else {
if (input != ctxt->input) {
xmlFatalErrMsg(ctxt, XML_ERR_SPACE_REQUIRED,
"PI declaration doesn't start and stop in the same entity\n");
if (inputid != ctxt->input->id) {
xmlFatalErrMsg(ctxt, XML_ERR_ENTITY_BOUNDARY,
"PI declaration doesn't start and stop in"
" the same entity\n");
}
SKIP(2);
@ -5311,7 +5319,7 @@ xmlParseNotationDecl(xmlParserCtxtPtr ctxt) {
xmlChar *Systemid;
if (CMP10(CUR_PTR, '<', '!', 'N', 'O', 'T', 'A', 'T', 'I', 'O', 'N')) {
xmlParserInputPtr input = ctxt->input;
int inputid = ctxt->input->id;
SHRINK;
SKIP(10);
if (!IS_BLANK_CH(CUR)) {
@ -5345,9 +5353,10 @@ xmlParseNotationDecl(xmlParserCtxtPtr ctxt) {
SKIP_BLANKS;
if (RAW == '>') {
if (input != ctxt->input) {
xmlFatalErrMsg(ctxt, XML_ERR_SPACE_REQUIRED,
"Notation declaration doesn't start and stop in the same entity\n");
if (inputid != ctxt->input->id) {
xmlFatalErrMsg(ctxt, XML_ERR_ENTITY_BOUNDARY,
"Notation declaration doesn't start and stop"
" in the same entity\n");
}
NEXT;
if ((ctxt->sax != NULL) && (!ctxt->disableSAX) &&
@ -5395,7 +5404,7 @@ xmlParseEntityDecl(xmlParserCtxtPtr ctxt) {
/* GROW; done in the caller */
if (CMP8(CUR_PTR, '<', '!', 'E', 'N', 'T', 'I', 'T', 'Y')) {
xmlParserInputPtr input = ctxt->input;
int inputid = ctxt->input->id;
SHRINK;
SKIP(8);
skipped = SKIP_BLANKS;
@ -5594,9 +5603,10 @@ xmlParseEntityDecl(xmlParserCtxtPtr ctxt) {
"xmlParseEntityDecl: entity %s not terminated\n", name);
xmlHaltParser(ctxt);
} else {
if (input != ctxt->input) {
if (inputid != ctxt->input->id) {
xmlFatalErrMsg(ctxt, XML_ERR_ENTITY_BOUNDARY,
"Entity declaration doesn't start and stop in the same entity\n");
"Entity declaration doesn't start and stop in"
" the same entity\n");
}
NEXT;
}
@ -5964,7 +5974,7 @@ xmlParseAttributeListDecl(xmlParserCtxtPtr ctxt) {
xmlEnumerationPtr tree;
if (CMP9(CUR_PTR, '<', '!', 'A', 'T', 'T', 'L', 'I', 'S', 'T')) {
xmlParserInputPtr input = ctxt->input;
int inputid = ctxt->input->id;
SKIP(9);
if (!IS_BLANK_CH(CUR)) {
@ -6060,10 +6070,10 @@ xmlParseAttributeListDecl(xmlParserCtxtPtr ctxt) {
GROW;
}
if (RAW == '>') {
if (input != ctxt->input) {
xmlValidityError(ctxt, XML_ERR_ENTITY_BOUNDARY,
"Attribute list declaration doesn't start and stop in the same entity\n",
NULL, NULL);
if (inputid != ctxt->input->id) {
xmlFatalErrMsg(ctxt, XML_ERR_ENTITY_BOUNDARY,
"Attribute list declaration doesn't start and"
" stop in the same entity\n");
}
NEXT;
}
@ -6100,10 +6110,10 @@ xmlParseElementMixedContentDecl(xmlParserCtxtPtr ctxt, int inputchk) {
SKIP_BLANKS;
SHRINK;
if (RAW == ')') {
if ((ctxt->validate) && (ctxt->input->id != inputchk)) {
xmlValidityError(ctxt, XML_ERR_ENTITY_BOUNDARY,
"Element content declaration doesn't start and stop in the same entity\n",
NULL, NULL);
if (ctxt->input->id != inputchk) {
xmlFatalErrMsg(ctxt, XML_ERR_ENTITY_BOUNDARY,
"Element content declaration doesn't start and"
" stop in the same entity\n");
}
NEXT;
ret = xmlNewDocElementContent(ctxt->myDoc, NULL, XML_ELEMENT_CONTENT_PCDATA);
@ -6159,10 +6169,10 @@ xmlParseElementMixedContentDecl(xmlParserCtxtPtr ctxt, int inputchk) {
}
if (ret != NULL)
ret->ocur = XML_ELEMENT_CONTENT_MULT;
if ((ctxt->validate) && (ctxt->input->id != inputchk)) {
xmlValidityError(ctxt, XML_ERR_ENTITY_BOUNDARY,
"Element content declaration doesn't start and stop in the same entity\n",
NULL, NULL);
if (ctxt->input->id != inputchk) {
xmlFatalErrMsg(ctxt, XML_ERR_ENTITY_BOUNDARY,
"Element content declaration doesn't start and"
" stop in the same entity\n");
}
SKIP(2);
} else {
@ -6402,10 +6412,10 @@ xmlParseElementChildrenContentDeclPriv(xmlParserCtxtPtr ctxt, int inputchk,
if (last != NULL)
last->parent = cur;
}
if ((ctxt->validate) && (ctxt->input->id != inputchk)) {
xmlValidityError(ctxt, XML_ERR_ENTITY_BOUNDARY,
"Element content declaration doesn't start and stop in the same entity\n",
NULL, NULL);
if (ctxt->input->id != inputchk) {
xmlFatalErrMsg(ctxt, XML_ERR_ENTITY_BOUNDARY,
"Element content declaration doesn't start and stop in"
" the same entity\n");
}
NEXT;
if (RAW == '?') {
@ -6578,7 +6588,7 @@ xmlParseElementDecl(xmlParserCtxtPtr ctxt) {
/* GROW; done in the caller */
if (CMP9(CUR_PTR, '<', '!', 'E', 'L', 'E', 'M', 'E', 'N', 'T')) {
xmlParserInputPtr input = ctxt->input;
int inputid = ctxt->input->id;
SKIP(9);
if (!IS_BLANK_CH(CUR)) {
@ -6644,9 +6654,10 @@ xmlParseElementDecl(xmlParserCtxtPtr ctxt) {
xmlFreeDocElementContent(ctxt->myDoc, content);
}
} else {
if (input != ctxt->input) {
if (inputid != ctxt->input->id) {
xmlFatalErrMsg(ctxt, XML_ERR_ENTITY_BOUNDARY,
"Element declaration doesn't start and stop in the same entity\n");
"Element declaration doesn't start and stop in"
" the same entity\n");
}
NEXT;
@ -6699,9 +6710,9 @@ xmlParseConditionalSections(xmlParserCtxtPtr ctxt) {
return;
} else {
if (ctxt->input->id != id) {
xmlValidityError(ctxt, XML_ERR_ENTITY_BOUNDARY,
"All markup of the conditional section is not in the same entity\n",
NULL, NULL);
xmlFatalErrMsg(ctxt, XML_ERR_ENTITY_BOUNDARY,
"All markup of the conditional section is not"
" in the same entity\n");
}
NEXT;
}
@ -6762,9 +6773,9 @@ xmlParseConditionalSections(xmlParserCtxtPtr ctxt) {
return;
} else {
if (ctxt->input->id != id) {
xmlValidityError(ctxt, XML_ERR_ENTITY_BOUNDARY,
"All markup of the conditional section is not in the same entity\n",
NULL, NULL);
xmlFatalErrMsg(ctxt, XML_ERR_ENTITY_BOUNDARY,
"All markup of the conditional section is not"
" in the same entity\n");
}
NEXT;
}
@ -6826,9 +6837,9 @@ xmlParseConditionalSections(xmlParserCtxtPtr ctxt) {
xmlFatalErr(ctxt, XML_ERR_CONDSEC_NOT_FINISHED, NULL);
} else {
if (ctxt->input->id != id) {
xmlValidityError(ctxt, XML_ERR_ENTITY_BOUNDARY,
"All markup of the conditional section is not in the same entity\n",
NULL, NULL);
xmlFatalErrMsg(ctxt, XML_ERR_ENTITY_BOUNDARY,
"All markup of the conditional section is not in"
" the same entity\n");
}
if ((ctxt-> instate != XML_PARSER_EOF) &&
((ctxt->input->cur + 3) <= ctxt->input->end))
@ -8382,12 +8393,26 @@ xmlParseInternalSubset(xmlParserCtxtPtr ctxt) {
/*
* Pop-up of finished entities.
*/
while ((RAW == 0) && (ctxt->inputNr > 1))
while (ctxt->inputNr > 1) {
if (RAW == 0) {
xmlPopInput(ctxt);
} else if (RAW == ']') {
/*
* Make sure not to return with unfinished entities.
*/
xmlFatalErr(ctxt, XML_ERR_EXT_SUBSET_NOT_FINISHED, NULL);
xmlPopInput(ctxt);
} else {
break;
}
}
if ((CUR_PTR == check) && (cons == ctxt->input->consumed)) {
xmlFatalErr(ctxt, XML_ERR_INTERNAL_ERROR,
"xmlParseInternalSubset: error detected in Markup declaration\n");
if (ctxt->inputNr > 1)
xmlPopInput(ctxt);
else
break;
}
}
@ -9282,7 +9307,7 @@ xmlParseStartTag2(xmlParserCtxtPtr ctxt, const xmlChar **pref,
xmlChar *attvalue;
const xmlChar **atts = ctxt->atts;
int maxatts = ctxt->maxatts;
int nratts, nbatts, nbdef;
int nratts, nbatts, nbdef, inputid;
int i, j, nbNs, attval;
unsigned long cur;
int nsNr = ctxt->nsNr;
@ -9299,6 +9324,7 @@ xmlParseStartTag2(xmlParserCtxtPtr ctxt, const xmlChar **pref,
*/
SHRINK;
cur = ctxt->input->cur - ctxt->input->base;
inputid = ctxt->input->id;
nbatts = 0;
nratts = 0;
nbdef = 0;
@ -9517,6 +9543,13 @@ next_attr:
GROW;
}
if (ctxt->input->id != inputid) {
xmlFatalErr(ctxt, XML_ERR_INTERNAL_ERROR,
"Unexpected change of input\n");
localname = NULL;
goto done;
}
/* Reconstruct attribute value pointers. */
for (i = 0, j = 0; j < nratts; i += 5, j++) {
if (atts[i+2] != NULL) {
@ -9675,6 +9708,7 @@ next_attr:
nsname, 0, NULL, nbatts / 5, nbdef, atts);
}
done:
/*
* Free up attribute allocated strings if needed
*/

17
result/errors/754946.xml.err
View File

@ -5,15 +5,16 @@ Entity: line 1: parser error : internal error: xmlParseInternalSubset: error det
Entity: line 1:
A<lbbbbbbbbbbbbbbbbbbb_
^
Entity: line 1: parser error : DOCTYPE improperly terminated
%SYSTEM;
./test/errors/754946.xml:4: parser error : internal error: xmlParseInternalSubset: error detected in Markup declaration
<![
^
Entity: line 1:
A<lbbbbbbbbbbbbbbbbbbb_
./test/errors/754946.xml:4: parser error : DOCTYPE improperly terminated
<![
^
Entity: line 1: parser error : Start tag expected, '<' not found
%SYSTEM;
./test/errors/754946.xml:4: parser error : StartTag: invalid element name
<![
^
Entity: line 1:
A<lbbbbbbbbbbbbbbbbbbb_
./test/errors/754946.xml:4: parser error : Extra content at the end of the document
<![
^

2
result/errors/754946.xml.str
View File

@ -1,4 +1,4 @@
./test/errors/754946.xml:1: parser error : Extra content at the end of the document
<!DOCTYPEA[<!ENTITY %
<!DOCTYPE A [
^
./test/errors/754946.xml : failed to parse

20
result/errors10/781205.xml.err
View File

@ -5,17 +5,13 @@ Entity: line 1: parser error : internal error: xmlParseInternalSubset: error det
Entity: line 1:
<:0000
^
Entity: line 1: parser error : DOCTYPE improperly terminated
%a;
^
Entity: line 1:
<:0000
^
namespace error : Failed to parse QName ':0000'
%a;
^
<:0000
^
./test/errors10/781205.xml:4: parser error : Couldn't find end of Start Tag :0000 line 1
./test/errors10/781205.xml:4: parser error : internal error: xmlParseInternalSubset: error detected in Markup declaration
^
./test/errors10/781205.xml:4: parser error : DOCTYPE improperly terminated
^
./test/errors10/781205.xml:4: parser error : Start tag expected, '<' not found
^

14
result/valid/t8.xml.err
View File

@ -5,15 +5,17 @@ Entity: line 1: parser error : internal error: xmlParseInternalSubset: error det
Entity: line 1:
&lt;!ELEMENT root (middle) >
^
Entity: line 1: parser error : DOCTYPE improperly terminated
%defroot;
Entity: line 1: parser error : internal error: xmlParseInternalSubset: error detected in Markup declaration
%defmiddle;
^
Entity: line 1:
&lt;!ELEMENT root (middle) >
&lt;!ELEMENT middle (test) >
^
Entity: line 1: parser error : Start tag expected, '<' not found
%defroot;
Entity: line 1: parser error : internal error: xmlParseInternalSubset: error detected in Markup declaration
%deftest;
^
Entity: line 1:
&lt;!ELEMENT root (middle) >
&lt;!ELEMENT test (#PCDATA) >
^

14
result/valid/t8.xml.err.rdr
View File

@ -5,10 +5,18 @@ Entity: line 1: parser error : internal error: xmlParseInternalSubset: error det
Entity: line 1:
&lt;!ELEMENT root (middle) >
^
Entity: line 1: parser error : DOCTYPE improperly terminated
%defroot;
Entity: line 1: parser error : internal error: xmlParseInternalSubset: error detected in Markup declaration
%defmiddle;
^
Entity: line 1:
&lt;!ELEMENT root (middle) >
&lt;!ELEMENT middle (test) >
^
Entity: line 1: parser error : internal error: xmlParseInternalSubset: error detected in Markup declaration
%deftest;
^
Entity: line 1:
&lt;!ELEMENT test (#PCDATA) >
^
./test/valid/t8.xml : failed to parse

14
result/valid/t8a.xml.err
View File

@ -5,15 +5,17 @@ Entity: line 1: parser error : internal error: xmlParseInternalSubset: error det
Entity: line 1:
&lt;!ELEMENT root (middle) >
^
Entity: line 1: parser error : DOCTYPE improperly terminated
%defroot;
Entity: line 1: parser error : internal error: xmlParseInternalSubset: error detected in Markup declaration
%defmiddle;
^
Entity: line 1:
&lt;!ELEMENT root (middle) >
&lt;!ELEMENT middle (test) >
^
Entity: line 1: parser error : Start tag expected, '<' not found
%defroot;
Entity: line 1: parser error : internal error: xmlParseInternalSubset: error detected in Markup declaration
%deftest;
^
Entity: line 1:
&lt;!ELEMENT root (middle) >
&lt;!ELEMENT test (#PCDATA) >
^

14
result/valid/t8a.xml.err.rdr
View File

@ -5,10 +5,18 @@ Entity: line 1: parser error : internal error: xmlParseInternalSubset: error det
Entity: line 1:
&lt;!ELEMENT root (middle) >
^
Entity: line 1: parser error : DOCTYPE improperly terminated
%defroot;
Entity: line 1: parser error : internal error: xmlParseInternalSubset: error detected in Markup declaration
%defmiddle;
^
Entity: line 1:
&lt;!ELEMENT root (middle) >
&lt;!ELEMENT middle (test) >
^
Entity: line 1: parser error : internal error: xmlParseInternalSubset: error detected in Markup declaration
%deftest;
^
Entity: line 1:
&lt;!ELEMENT test (#PCDATA) >
^
./test/valid/t8a.xml : failed to parse

5
test/errors/754946.xml
View File

@ -1 +1,4 @@
<!DOCTYPEA[<!ENTITY % SYSTEM "A<lbbbbbbbbbbbbbbbbbbb_" >%SYSTEM;<![
<!DOCTYPE A [
<!ENTITY % SYSTEM "A<lbbbbbbbbbbbbbbbbbbb_">
%SYSTEM;
<![