c01de718b2fb8e7706a0fede0e5ea4149ac6f378 braney Mon Aug 31 13:48:13 2026 -0700 htmlSanitize: six fixes from a second review of it, refs #38126 Filter the style attribute on the text a browser will see. A browser turns a character reference into the character it names before the CSS parser runs, so a value spelled url( reached the page as url( and walked past the check that is there to stop it. The value is decoded before the check now, and the author is told which property lost its value. Say something when we drop the rest of the page. A tag that never ends, most often an attribute value whose quote is never closed, threw away everything after it and reported nothing, so hubCheck stayed quiet about it. The depth cap did the same. Write a less than sign that starts no tag as <. A browser reads text like "</ " as a comment and swallows markup of ours up to the next '>', which could eat a closing tag we added. Write an attribute once. A browser keeps the first of a repeated attribute and drops the rest. We checked the first and then printed them all, which was only correct because of that rule. Do not stack a second descPage- prefix on an id we already renamed, and do not add a second noopener noreferrer to a rel that already has one. hgCustom hands the text we returned back to us when a custom track is edited and saved again, so both of these grew a little more on every save. The test now re-runs the filter over its own output and prints any case that changes, so a transform that compounds shows up in the diff. diff --git src/lib/tests/htmlSanitizeTest.c src/lib/tests/htmlSanitizeTest.c index 5f18da8167a..cd081f855cd 100644 --- src/lib/tests/htmlSanitizeTest.c +++ src/lib/tests/htmlSanitizeTest.c @@ -38,37 +38,63 @@ "<div style=\"color:red\"/>the rest of the page is not inside that div", /* a page built of tags that are never closed is not a way to make us work all day */ "<svg><svg><svg><svg>text", /* a stray quote inside an unquoted value does not swallow the page */ "<a href=https://example.com/x\\\">link text</a> and more text", /* a form and everything in it goes */ "<form action=\"/x\"><input name=\"password\"><button>Log in</button></form><p>after</p>", /* comments and doctypes go */ "<!DOCTYPE html><!-- <p>hidden</p> --><p>shown</p>", /* an id, and a name on an anchor, are renamed, and a link to one of them is renamed too */ "<h2 id=\"methods\">M</h2><a name=\"top\">t</a>" "<a href=\"#methods\">same page</a> <a href=\"other.html#methods\">other page</a>" "<a href=\"#\">to the top</a><div id=\"\">no name at all</div>", /* a table keeps its shape */ "<table border=\"1\"><tr><td colspan=\"2\" bgcolor=\"#eee\">cell</td></tr></table>", +/* an entity in a style value cannot spell a property value we would not print */ +"<p style=\"list-style:url(http://example.com/x.png);color:red\">a</p>", +/* an ampersand the author meant still reads as one */ +"<p style=\"font-family:'AT&T Sans'\">a</p>", +/* the same attribute twice is written once, the way a browser reads it */ +"<img src=\"first.png\" src=\"second.png\" alt=\"a\" alt=\"b\">" + "<a href=\"javascript:alert(1)\" href=\"https://example.com\">x</a>", +/* a quote that is never closed takes the rest of the page, and we say so */ +"<p>real</p><p title=\"oops>never printed</p>", +/* a less than sign that starts no tag is text, and cannot eat a tag of ours */ +"<div>a < b</ <b>bold</b></div>", +/* a name we already renamed is not renamed again, on either side of the link */ +"<h2 id=\"descPage-methods\">M</h2><a href=\"#descPage-methods\">jump</a>", }; int main(int argc, char *argv[]) { int i; for (i = 0; i < ArraySize(cases); ++i) { char *clean = htmlSanitize(cases[i]); printf("in : %s\nout: %s\n", cases[i], clean); struct slName *removed = NULL, *el; freeMem(clean); clean = htmlSanitizeReport(cases[i], &removed); for (el = removed; el != NULL; el = el->next) printf(" (%s)\n", el->name); printf("\n"); freeMem(clean); slFreeList(&removed); } +/* Running the filter over its own output has to leave it alone. hgCustom hands the text + * we returned back to us when a custom track is edited and saved again, so anything that + * changes on a second pass changes a little more on every save. Any case that is not + * settled prints here, and the expected output is the record of which ones those are. */ +for (i = 0; i < ArraySize(cases); ++i) + { + char *once = htmlSanitize(cases[i]); + char *twice = htmlSanitize(once); + if (differentString(once, twice)) + printf("not settled:\n once : %s\n twice: %s\n\n", once, twice); + freeMem(once); + freeMem(twice); + } if (htmlSanitize(NULL) != NULL) errAbort("htmlSanitize(NULL) should be NULL"); return 0; }