3842ba31ab0b697b0d4026b5751da7dac7c84e35 braney Tue Sep 29 13:49:49 2026 -0700 htmlSanitize: keep the stdTbl and copyLinkSpan class names, refs #38126 GenArk description pages use these two classes from our own stylesheet and scripts: stdTbl for bordered tables, and copyLinkSpan with data-target for the Copy button next to the share link. They now come through, and data-target gets the same prefix as the id it names. Style values are now read the same way on every pass, so a page that is saved again comes out unchanged. Over the 5390-page hub corpus, 278 pages change, all in class or data-target only, and no page's text changes. diff --git src/lib/htmlSanitize.c src/lib/htmlSanitize.c index 32ac7a7b833..8c489f2cda0 100644 --- src/lib/htmlSanitize.c +++ src/lib/htmlSanitize.c @@ -67,47 +67,47 @@ /* The svg elements that mean something only inside an svg. */ static char *svgShapeElements = "g circle ellipse rect line polyline polygon path"; /* Attributes that color and place a shape, allowed on every svg element. */ #define svgPaintAttrs "fill fill-opacity fill-rule stroke stroke-width stroke-opacity " \ "stroke-linecap stroke-linejoin stroke-dasharray opacity transform" /* Attributes allowed, by element. The first row is for every kept element. */ struct attrRule { char *element; /* Element name, or "*" for all of them. */ char *attrs; /* Space separated attribute names. */ }; static struct attrRule attrRules[] = { - {"*", "title dir lang style id"}, + {"*", "title dir lang style id class"}, {"a", "href target rel name"}, {"img", "src alt width height border align hspace vspace"}, {"table", "width border cellpadding cellspacing align bgcolor summary"}, {"td", "colspan rowspan align valign width height nowrap bgcolor scope"}, {"th", "colspan rowspan align valign width height nowrap bgcolor scope"}, {"tr", "align valign bgcolor"}, {"col", "span width align valign"}, {"colgroup", "span width align valign"}, {"ol", "start type reversed"}, {"ul", "type"}, {"li", "type value"}, {"font", "color face size"}, {"hr", "width size align noshade"}, {"p", "align"}, {"div", "align"}, - {"span", "align"}, + {"span", "align data-target"}, {"h1", "align"}, {"h2", "align"}, {"h3", "align"}, {"h4", "align"}, {"h5", "align"}, {"h6", "align"}, {"caption", "align"}, {"thead", "align valign"}, {"tbody", "align valign"}, {"tfoot", "align valign"}, {"iframe", "src width height frameborder allowfullscreen allow loading"}, {"svg", "width height viewbox preserveaspectratio " svgPaintAttrs}, {"g", svgPaintAttrs}, {"circle", "cx cy r " svgPaintAttrs}, {"ellipse", "cx cy rx ry " svgPaintAttrs}, @@ -121,72 +121,79 @@ /* Properties allowed inside a style attribute. */ static char *styleProperties = "text-align text-align-last vertical-align white-space word-break word-wrap overflow-wrap " "padding padding-top padding-bottom padding-left padding-right " "margin margin-top margin-bottom margin-left margin-right " "border border-top border-bottom border-left border-right border-color border-style " "border-width border-radius border-collapse border-spacing " "width height min-width max-width min-height max-height " "color background-color " "font font-size font-weight font-style font-family font-variant " "line-height letter-spacing text-decoration text-transform text-indent " "list-style list-style-type list-style-position " "float clear display overflow overflow-x overflow-y opacity " "fill fill-opacity stroke stroke-width stroke-opacity"; +/* Class names allowed. A class from outside would pick up whatever our own stylesheets and + * scripts do with that name, so only names we want that for get through. stdTbl is the + * bordered table in HGStyle.css, and copyLinkSpan is where hgGateway.js puts a copy button + * for the link named by the span's data-target. Our own GenArk description pages use both. */ +static char *classNames = "stdTbl copyLinkSpan"; + /* URL schemes allowed in href and src. A URL with no scheme at all is allowed too. */ static char *urlSchemes = "http https mailto ftp"; /* Hosts an iframe may point at. */ static char *videoHosts = "www.youtube.com youtube.com www.youtube-nocookie.com youtube-nocookie.com youtu.be " "player.vimeo.com vimeo.com"; /* Nesting past this depth is not a document, it is a way to make us emit a huge page. */ #define maxNestDepth 256 /* The longest attribute name we look at, and the most attributes we remember writing on one * tag. Every name on the allowlist is well inside both. */ #define maxAttrName 128 #define maxTagAttrs 32 static struct hash *keepHash = NULL, *killHash = NULL, *voidHash = NULL, *rawTextHash = NULL; -static struct hash *silentKillHash = NULL, *svgShapeHash = NULL; +static struct hash *silentKillHash = NULL, *svgShapeHash = NULL, *classNameHash = NULL; static struct hash *attrHash = NULL, *stylePropHash = NULL, *schemeHash = NULL, *videoHostHash = NULL; static struct hash *hashOfWords(char *words, int sizePow2) /* Return a hash holding each space separated word in words. */ { struct hash *hash = hashNew(sizePow2); char *dupe = cloneString(words); char *word, *s = dupe; while ((word = nextWord(&s)) != NULL) hashAdd(hash, word, NULL); freeMem(dupe); return hash; } static void initTables() /* Build the lookup hashes on first use. */ { if (keepHash != NULL) return; killHash = hashOfWords(killElements, 7); silentKillHash = hashOfWords(silentKillElements, 5); voidHash = hashOfWords(voidElements, 6); rawTextHash = hashOfWords(rawTextElements, 5); svgShapeHash = hashOfWords(svgShapeElements, 4); +classNameHash = hashOfWords(classNames, 3); stylePropHash = hashOfWords(styleProperties, 8); schemeHash = hashOfWords(urlSchemes, 4); videoHostHash = hashOfWords(videoHosts, 4); attrHash = hashNew(9); int i; for (i = 0; i < ArraySize(attrRules); ++i) { char *dupe = cloneString(attrRules[i].attrs); char *attr, *s = dupe; while ((attr = nextWord(&s)) != NULL) { char key[256]; safef(key, sizeof key, "%s.%s", attrRules[i].element, attr); hashAdd(attrHash, key, NULL); } @@ -551,75 +558,199 @@ static void stripCssComments(char *s) /* Blank out CSS comments in place. */ { char *open; while ((open = stringIn("/*", s)) != NULL) { char *close = stringIn("*/", open+2); char *end = (close == NULL ? open + strlen(open) : close+2); while (open < end) *open++ = ' '; s = end; } } +/* Named character references we let through in a style value. They are the ones our own + * escaping writes, so the filter has to read its own output back unchanged, and none of them + * can spell anything a browser would act on. */ +static char *safeEntityNames[] = {"quot", "amp", "apos", "lt", "gt"}; + +static int entityNameLen(char *s) +/* s points just past an ampersand. Return the length of the entity name there, or 0. */ +{ +if (!isalpha((unsigned char)*s)) + return 0; +int len = 1; +while (isalnum((unsigned char)s[len])) + ++len; +return len; +} + +static boolean isSafeEntityName(char *s, int len) +/* Is the name of length len at s one of safeEntityNames? */ +{ +int i; +for (i = 0; i < ArraySize(safeEntityNames); ++i) + { + if (strlen(safeEntityNames[i]) == len && strncmp(s, safeEntityNames[i], len) == 0) + return TRUE; + } +return FALSE; +} + +static char *referenceSemicolon(char *amp) +/* amp points at an ampersand. If it starts a character reference whose semicolon a browser + * takes as part of the reference, return that semicolon, otherwise NULL. A number always + * counts. A name counts only when it is one we let through, because a name a browser does + * not know leaves its semicolon to end the declaration. */ +{ +char *p = amp + 1; +if (*p == '#') + { + p += 1; + char *digits; + if (*p == 'x' || *p == 'X') + { + digits = ++p; + while (isxdigit((unsigned char)*p)) + ++p; + } + else + { + digits = p; + while (isdigit((unsigned char)*p)) + ++p; + } + return (p > digits && *p == ';') ? p : NULL; + } +int len = entityNameLen(p); +if (len > 0 && p[len] == ';' && isSafeEntityName(p, len)) + return p + len; +return NULL; +} + +static char *declarationEnd(char *s) +/* Return the semicolon that ends the declaration starting at s, or NULL if it runs to the + * end. The semicolon of a character reference is part of the value. Our own output writes + * a quote as ", so splitting there would cut apart a declaration we already allowed. */ +{ +char *p; +for (p = s; *p != 0; ++p) + { + if (*p == '&') + { + char *semi = referenceSemicolon(p); + if (semi != NULL) + p = semi; + } + else if (*p == ';') + return p; + } +return NULL; +} + +static boolean hasOtherEntityName(char *value) +/* Does value hold a named reference other than the safe ones? A browser decodes a name it + * knows before the CSS parser runs, and some decode to a parenthesis or a backslash. A name + * at the very end counts even without its semicolon, because the semicolon ended the + * declaration and we write one back after it. */ +{ +char *p; +for (p = strchr(value, '&'); p != NULL; p = strchr(p+1, '&')) + { + int len = entityNameLen(p+1); + if (len > 0 && (p[len+1] == ';' || p[len+1] == 0) && !isSafeEntityName(p+1, len)) + return TRUE; + } +return FALSE; +} + +static boolean endsInOpenReference(char *value) +/* Does value end in a reference with no semicolon of its own, one a browser reads the same + * with or without it? The semicolon we write after the value would then close it, and the + * next pass would read that semicolon as part of the value. */ +{ +char *amp = strrchr(value, '&'); +if (amp == NULL) + return FALSE; +char *p = amp + 1; +if (*p == '#') + { + p += 1; + boolean hex = (*p == 'x' || *p == 'X'); + if (hex) + p += 1; + char *digits = p; + while (hex ? isxdigit((unsigned char)*p) : isdigit((unsigned char)*p)) + ++p; + return (p > digits && *p == 0); + } +int len = entityNameLen(p); +return (len > 0 && p[len] == 0 && isSafeEntityName(p, len)); +} + static boolean cssValueOk(char *value) /* Is value one we will print as a style property, or as an svg attribute that takes the * same values? Look at the text a browser will see, not the text the author wrote. A * browser turns a character reference into the character it names before the CSS parser - * runs, so url( would otherwise walk past the check below. */ + * runs, so url( would otherwise walk past the check below. Numbers are decoded here. + * A name other than the safe ones is refused outright. A name without its semicolon is + * decoded only from a legacy handful that all name Latin-1 punctuation, which cannot spell + * anything. */ { +if (hasOtherEntityName(value)) + return FALSE; char *lower = decodeNumericRefs(value); tolowers(lower); boolean ok = (stringIn("url(", lower) == NULL && stringIn("expression", lower) == NULL && strchr(lower, '\\') == NULL); freeMem(lower); return ok; } static char *filterStyle(char *val, struct sanitizer *san) /* Return the declarations of val that we allow, or NULL if none of them survive. */ { char *dupe = cloneString(val); stripCssComments(dupe); struct dyString *out = dyStringNew(strlen(dupe)+1); char *decl = dupe; while (decl != NULL && *decl != 0) { - char *next = strchr(decl, ';'); + char *next = declarationEnd(decl); if (next != NULL) *next++ = 0; char *colon = strchr(decl, ':'); if (colon != NULL) { *colon = 0; char *prop = trimSpaces(decl); char *value = trimSpaces(colon+1); tolowers(prop); if (isNotEmpty(prop) && isNotEmpty(value)) { - /* Only the numeric form of a character reference needs decoding here. A - * named reference has to end in a semicolon, apart from a legacy handful that - * all name Latin-1 punctuation, and a semicolon has already ended the - * declaration before we get here. */ if (hashLookup(stylePropHash, prop) == NULL) ; /* not a property we print, and nothing to explain */ else if (!cssValueOk(value)) noteRemoved(san, "removed the value of the style property %s", prop); else - dyStringPrintf(out, "%s:%s;", prop, value); + { + /* Close an open reference, so the output reads back the same. */ + dyStringPrintf(out, "%s:%s%s;", prop, value, + endsInOpenReference(value) ? ";" : ""); + } } } decl = next; } freeMem(dupe); if (out->stringSize == 0) { dyStringFree(&out); return NULL; } return dyStringCannibalize(&out); } static char *skipIdPrefix(char *name) /* Return name past a prefix we put there ourselves. Adding a second one would work, but @@ -705,30 +836,58 @@ if (trimmed[0] == '#' && trimmed[1] != 0) fragment = trimmed + 1; /* a link to a name on this same page */ } dyStringPrintf(san->out, " %s=\"", attr); if (fragment != NULL) { /* The name it points at is being renamed, so rename this to match. */ dyStringAppend(san->out, "#" htmlSanitizeIdPrefix); appendEscaped(san->out, skipIdPrefix(fragment)); } else appendEscaped(san->out, value); dyStringAppendC(san->out, '"'); } } + else if (sameString(attr, "class")) + { + /* Keep the names on our list and drop the rest. */ + struct dyString *kept = dyStringNew(0); + char *word, *rest = value; + while ((word = nextWord(&rest)) != NULL) + { + if (hashLookup(classNameHash, word) != NULL) + { + if (kept->stringSize > 0) + dyStringAppendC(kept, ' '); + dyStringAppend(kept, word); + } + } + if (kept->stringSize > 0) + dyStringPrintf(san->out, " class=\"%s\"", kept->string); + dyStringFree(&kept); + } + else if (sameString(attr, "data-target")) + { + /* It names an id on this same page, and that id is being renamed. */ + if (isNotEmpty(value)) + { + dyStringAppend(san->out, " data-target=\"" htmlSanitizeIdPrefix); + appendEscaped(san->out, skipIdPrefix(value)); + dyStringAppendC(san->out, '"'); + } + } else if (sameString(attr, "id") || (isAnchor && sameString(attr, "name"))) { /* An id here lands in a page of ours, next to ids our own JavaScript looks up, * and it also becomes a property of that name on window. A prefix keeps the two * sets apart. An anchor name does both of those things too. */ if (isNotEmpty(value)) { dyStringPrintf(san->out, " %s=\"%s", attr, htmlSanitizeIdPrefix); appendEscaped(san->out, skipIdPrefix(value)); dyStringAppendC(san->out, '"'); } } else if (isAnchor && sameString(attr, "rel")) relValue = cloneString(value); /* a repeat of it never reaches here */ else if (isSvg && !cssValueOk(value))