Skip to content

Commit

Permalink
CVE-2015-7500 Fix memory access error due to incorrect entities bound…
Browse files Browse the repository at this point in the history
…aries

For https://bugzilla.gnome.org/show_bug.cgi?id=756525
handle properly the case where we popped out of the current entity
while processing a start tag
Reported by Kostya Serebryany @ Google

This slightly modifies the output of 754946 in regression tests
  • Loading branch information
veillard committed Nov 20, 2015
1 parent fdfeecc commit f1063fd
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 8 deletions.
28 changes: 22 additions & 6 deletions parser.c
Expand Up @@ -9348,7 +9348,7 @@ xmlParseStartTag2(xmlParserCtxtPtr ctxt, const xmlChar **pref,
const xmlChar **atts = ctxt->atts;
int maxatts = ctxt->maxatts;
int nratts, nbatts, nbdef;
int i, j, nbNs, attval, oldline, oldcol;
int i, j, nbNs, attval, oldline, oldcol, inputNr;
const xmlChar *base;
unsigned long cur;
int nsNr = ctxt->nsNr;
Expand All @@ -9367,6 +9367,7 @@ xmlParseStartTag2(xmlParserCtxtPtr ctxt, const xmlChar **pref,
SHRINK;
base = ctxt->input->base;
cur = ctxt->input->cur - ctxt->input->base;
inputNr = ctxt->inputNr;
oldline = ctxt->input->line;
oldcol = ctxt->input->col;
nbatts = 0;
Expand All @@ -9392,7 +9393,8 @@ xmlParseStartTag2(xmlParserCtxtPtr ctxt, const xmlChar **pref,
*/
SKIP_BLANKS;
GROW;
if (ctxt->input->base != base) goto base_changed;
if ((ctxt->input->base != base) || (inputNr != ctxt->inputNr))
goto base_changed;

while (((RAW != '>') &&
((RAW != '/') || (NXT(1) != '>')) &&
Expand All @@ -9403,7 +9405,7 @@ xmlParseStartTag2(xmlParserCtxtPtr ctxt, const xmlChar **pref,

attname = xmlParseAttribute2(ctxt, prefix, localname,
&aprefix, &attvalue, &len, &alloc);
if (ctxt->input->base != base) {
if ((ctxt->input->base != base) || (inputNr != ctxt->inputNr)) {
if ((attvalue != NULL) && (alloc != 0))
xmlFree(attvalue);
attvalue = NULL;
Expand Down Expand Up @@ -9552,7 +9554,8 @@ xmlParseStartTag2(xmlParserCtxtPtr ctxt, const xmlChar **pref,
break;
}
SKIP_BLANKS;
if (ctxt->input->base != base) goto base_changed;
if ((ctxt->input->base != base) || (inputNr != ctxt->inputNr))
goto base_changed;
continue;
}

Expand Down Expand Up @@ -9589,7 +9592,8 @@ xmlParseStartTag2(xmlParserCtxtPtr ctxt, const xmlChar **pref,
GROW
if (ctxt->instate == XML_PARSER_EOF)
break;
if (ctxt->input->base != base) goto base_changed;
if ((ctxt->input->base != base) || (inputNr != ctxt->inputNr))
goto base_changed;
if ((RAW == '>') || (((RAW == '/') && (NXT(1) == '>'))))
break;
if (!IS_BLANK_CH(RAW)) {
Expand All @@ -9605,7 +9609,8 @@ xmlParseStartTag2(xmlParserCtxtPtr ctxt, const xmlChar **pref,
break;
}
GROW;
if (ctxt->input->base != base) goto base_changed;
if ((ctxt->input->base != base) || (inputNr != ctxt->inputNr))
goto base_changed;
}

/*
Expand Down Expand Up @@ -9772,6 +9777,17 @@ xmlParseStartTag2(xmlParserCtxtPtr ctxt, const xmlChar **pref,
if ((ctxt->attallocs[j] != 0) && (atts[i] != NULL))
xmlFree((xmlChar *) atts[i]);
}

/*
* We can't switch from one entity to another in the middle
* of a start tag
*/
if (inputNr != ctxt->inputNr) {
xmlFatalErrMsg(ctxt, XML_ERR_ENTITY_BOUNDARY,
"Start tag doesn't start and stop in the same entity\n");
return(NULL);
}

ctxt->input->cur = ctxt->input->base + cur;
ctxt->input->line = oldline;
ctxt->input->col = oldcol;
Expand Down
7 changes: 5 additions & 2 deletions result/errors/754946.xml.err
Expand Up @@ -11,6 +11,9 @@ Entity: line 1: parser error : DOCTYPE improperly terminated
Entity: line 1:
A<lbbbbbbbbbbbbbbbbbbb_
^
./test/errors/754946.xml:1: parser error : Start tag doesn't start and stop in the same entity
>%SYSTEM;<![
^
./test/errors/754946.xml:1: parser error : Extra content at the end of the document
<!DOCTYPEA[<!ENTITY %
^
>%SYSTEM;<![
^

0 comments on commit f1063fd

Please sign in to comment.