Skip to content

Commit

Permalink
Revert "Do not URI escape in server side includes"
Browse files Browse the repository at this point in the history
This reverts commit 960f0e2.

This commit introduced

- an infinite loop, found by OSS-Fuzz, which could be easily fixed.
- an algorithm with quadratic runtime
- a security issue, see
  https://bugzilla.gnome.org/show_bug.cgi?id=769760

A better approach is to add an option not to escape URLs at all
which libxml2 should have possibly done in the first place.
  • Loading branch information
nwellnhof committed Aug 15, 2020
1 parent b82fa3d commit c1ba6f5
Showing 1 changed file with 11 additions and 38 deletions.
49 changes: 11 additions & 38 deletions HTMLtree.c
Original file line number Diff line number Diff line change
Expand Up @@ -706,49 +706,22 @@ htmlAttrDumpOutput(xmlOutputBufferPtr buf, xmlDocPtr doc, xmlAttrPtr cur,
(!xmlStrcasecmp(cur->name, BAD_CAST "src")) ||
((!xmlStrcasecmp(cur->name, BAD_CAST "name")) &&
(!xmlStrcasecmp(cur->parent->name, BAD_CAST "a"))))) {
xmlChar *escaped;
xmlChar *tmp = value;
/* xmlURIEscapeStr() escapes '"' so it can be safely used. */
xmlBufCCat(buf->buffer, "\"");

while (IS_BLANK_CH(*tmp)) tmp++;

/* URI Escape everything, except server side includes. */
for ( ; ; ) {
xmlChar *escaped;
xmlChar endChar;
xmlChar *end = NULL;
xmlChar *start = (xmlChar *)xmlStrstr(tmp, BAD_CAST "<!--");
if (start != NULL) {
end = (xmlChar *)xmlStrstr(tmp, BAD_CAST "-->");
if (end != NULL) {
*start = '\0';
}
}

/* Escape the whole string, or until start (set to '\0'). */
escaped = xmlURIEscapeStr(tmp, BAD_CAST"@/:=?;#%&,+");
if (escaped != NULL) {
xmlBufCat(buf->buffer, escaped);
xmlFree(escaped);
} else {
xmlBufCat(buf->buffer, tmp);
}

if (end == NULL) { /* Everything has been written. */
break;
}

/* Do not escape anything within server side includes. */
*start = '<'; /* Restore the first character of "<!--". */
end += 3; /* strlen("-->") */
endChar = *end;
*end = '\0';
xmlBufCat(buf->buffer, start);
*end = endChar;
tmp = end;
/*
* the < and > have already been escaped at the entity level
* And doing so here breaks server side includes
*/
escaped = xmlURIEscapeStr(tmp, BAD_CAST"@/:=?;#%&,+<>");
if (escaped != NULL) {
xmlBufWriteQuotedString(buf->buffer, escaped);
xmlFree(escaped);
} else {
xmlBufWriteQuotedString(buf->buffer, value);
}

xmlBufCCat(buf->buffer, "\"");
} else {
xmlBufWriteQuotedString(buf->buffer, value);
}
Expand Down

0 comments on commit c1ba6f5

Please sign in to comment.