Give a free option or list item its wrapper under every container, and push the select out of a kept table - #497

Item 9 of #492: an option under a dropped table cell is emitted without its select and is not a fixed point.

What broke

The issue's shape, <table><th><option>tail under a policy that drops the table parts, has been a fixed point since a late #490 commit worked around it for option in a dropped cell. Its class had not gone away. On main, under a policy that keeps select, option and optgroup and drops the table parts, <table><th><optgroup>tail gives <optgroup>tail</optgroup> and the next pass <select><optgroup>tail</optgroup></select>; <table><tr><td><optgroup>a</optgroup><option>b gives <optgroup>a</optgroup><select><option>b</option></select> and then a select inside a select. With the table kept and only the cell dropped, <table><th><option>tail gives <table><tbody><tr><select><option>tail</option></select></tr></tbody></table>, a select inside a row, and the next pass moves it out. With everything allowed, <ul><option>x gives <ul><li><option>x</option></li></ul> and the next pass adds the select, while <ul><li><option>x gets it at once; <span><option>x, <u><optgroup>x, <td>x<option>y and <var><li>x get no wrapper at all. Under a policy that drops template, <u><template><option>x gives <u><option>x</option></u> and the next pass wraps it. The probe on main flags 7,307 non-idempotent rows whose input holds an option or optgroup.

Why

HtmlElementTables.impliedElements wraps a free option or optgroup in a select, a free li in a ul, and a free cell in table structure, under any container that is not one of the few that hold the element directly. Those few are kept as a boolean[] only as long as its highest member, and the check read an ancestor whose index lies past the end of the array as one of them. Element indices are alphabetical, so every name after select (span, td, th, tt, u, ul, var, ...) was an "allowed container" for an option and every name after ul (var, video) for a list item. Under a ul the list's own rule then implied an li, and the next pass, seeing the option in the li, applied the wrapper: not a fixed point. The comment above the code says what was meant: "we make sure we never produce an <li> or <td> without a corresponding LIST or TABLE scope element on the stack".

Two more things stood between the wrapper and a fixed point once the bound was right. The balancer judged the option against its logical container, so a cell the policy dropped inside a kept row held the select in the output's row, and a template the policy dropped held the option bare on the first pass only. And template really does hold an option directly, so the wrapper's set has to name it.

What changed

• HtmlElementTables.impliedElements: an ancestor past the end of the select and ul wrappers' sets is not in them. The table-part wrappers keep their old reading of an ancestor past the end, under a flag named for that (pastEndCountsAsAllowed): the probe showed that changing them moves text out of forms and nests table parts differently on hundreds of inputs, and two existing tests pin their behaviour, so that is left alone and noted in the code. template joins the option and optgroup wrappers' sets, as a browser's template holds an option directly.
• TagBalancingHtmlStreamEventReceiver: an option or optgroup under a template the policy dropped or renamed is judged against the output's container, where the option lands, instead of the template. The select implied for an option under entries the policy dropped, a cell, a caption, a template or an inline element, whatever their names and however many, is judged by the nearest entry below with output: if that is table structure, which holds no select, the table is pushed out from that entry, as a browser foster-parents the select before the table, while the dropped entries stay where they are, so that the cell's end tag still closes the option it holds and text after the cell is not the option's. A container renamed into table structure, and not table structure itself, is closed instead. One helper reads the output's container for the three rules that ask for it, and one finds the start of an implied path after its container for the three loops that did.
• Tests: HtmlElementTablesTest.testImpliedElements pins the wrapper under ul, span, td and var, the containers that hold the element directly, and the table parts' unchanged reading; HtmlSanitizerTest covers the shapes above under a permissive policy, under the issue's table-dropping policy, under a policy that keeps the table and drops only the cell (dropped inline elements and a dropped template between the cell and the option, text after the cell's end tag, a cell after the option), under policies that drop, rename (to u and to table) or keep the template, and inside a select, with hostile payloads: <style> and <script> inside an option, which stay text as CVE-2021-42575 requires, an img with an event handler, and a javascript: link under a policy that allows links so that the URL policy is what removes it. Existing tests are unchanged.
• change_log.md bullet.

Before and after

Every output is a fixed point. The prepackaged policies allow no option, optgroup or li without their containers being allowed too, so nothing in Sanitizers widens.

Verification

• ./mvnw -o -ntp -B clean verify on the final commit 8b6c865, with main (Keep the list's implied item out of a forwarded foreign root #495 merged) merged in: build success, 686 tests, 0 failures; existing tests unchanged.
• Differential probe (probes-review3/ReviewProbe, 15,430 inputs x 44 policies, san and events modes, main 75ab9c0 (with Keep the list's implied item out of a forwarded foreign root #495) against the final commit, which has that main merged in):
  • san: 678,920 rows compared. 10,372 differ in output. 4,176 rows lose every flag (non-idempotent and tree-mutates under the policies that drop template, drop table parts, drop raw-text elements or disallow text somewhere; the option's wrapper on the first pass), 596 keep fewer flags, no row throws, and 28 rows on 13 distinct inputs gain one. Each is a consequence of the wrapper now applying under a container that was past the bound:
  • fidelity (the output's browser tree differs from the input's), 8 rows on 8 inputs, all under the policy that allows everything: an option or optgroup under u, strong, th, o:p, foo, or a span inside a select, gets the select a browser does not make there. The same normalisation <div><option>x had on main.
  • text-changed, 15 rows on 1 input: <u>...y<optgroup>...<svg><o:p><textArea>... under every policy that keeps select; the optgroup's select now holds the rest of the input, where the output parser's select rules drop the start tags a select cannot hold, so the browser text of the output changes though the sanitizer's text does not.
  • text-in-form and text-in-foo, 3 rows on 3 inputs, under the policies that disallow text in form or in foo: two are an option whose implied select is closed by a stray </select> later in the input, which puts a following form outside the SVG root that main kept it in; one is <style> inside an option inside a form in a table, where the select's rule that a style inside it holds no text turns the stylesheet into escaped text. Both rules applied under div on main.
  • Not a fixed point on the first pass, 2 rows on 1 input, under Sanitizers.TABLES: an option under a dropped template in a table holds a noscript with a col that gets no table on main either (<option><noscript><col> gives a bare <col /> on main), and with the select pushed out of the table the bare col now lands after it instead of inside it.
  • events (the receiver alone, at nesting limits 0..8): 36,063 rows, 278 differ, 47 lose their limit flags, 3 gain one: at limit 8 the option's select is one more level, so a list item for text that fit on main is dropped at the limit and added by the second pass at the default limit; at a dynamic limit of 6 the extra level makes setNestingLimit throw its documented IllegalStateException where main had one level fewer open.
  • Earlier runs of the full bound fix regressed hundreds of rows (text out of forms, nested table parts, template fidelity); narrowing it to the select and list wrappers, and judging the dropped template and cell by the output, brought it to the above. The run before the reviews' fixes had 30 rows on 14 inputs against the previous main; a first version of the set-aside threw on every option under a dropped template inside a select, which the probe caught and a test now pins; the final push-out has the same 28 rows as the set-aside had.

Review

/code-review found the select still emitted inside the row when a dropped template or a dropped inline element stood between the dropped cell and the option (the check looked at the immediate container's name), the dropped cell's stack entry removed before the select was implied, so that its end tag matched nothing and text after the cell was folded into the option, a template renamed into table structure judged by the output for the wrapper but by the stack for where the select fits, the change log claiming a list under a dropped list, tests missing those shapes and a hostile link the policy did not allow anyway, three copies of the output-container lookup, and a wrapper flag named for the element rather than what it does. All fixed above, each with a test where behaviour changed: the nearest emitted ancestor decides, the dropped entries are set aside and put back, the renamed container is closed, the claim is narrowed, one helper does the lookup, and the flag is pastEndCountsAsAllowed. Its suggestion to route the implied select through the policy's prepared-start path instead of the special case is left for the follow-up that reworks the implied-element loop.

A second /code-review found the set-aside itself unsound: the dropped entries were stacked again without their open events, so a nested cell's end tag could match an outer cell in the policy; entries above the container that containerIndex skips were closed and not restored; per-entry state (form pointer marks, implied-table bits) was lost; closeStackFrom queued the dropped formatting for resumption and cleared the resume queue for a template; forwarded elements inside the cell were closed early; and the renamed-container branch fired for a genuine row. It suggested pushing the table out directly from the emitted entry, which is what the branch now does, with the dropped entries untouched; the other findings, the missing foreign guard on the dropped-template rule, the copy-pasted start-position loop, the dangling doc comment, the tautological tree assertions and the test comment, are fixed above. Its wish to size the wrappers' sets to the element count instead of keeping the table parts' past-end reading is the same change the first probe run rejected, and stays out.

Left out

• An option in a table cell now gets a select like an option anywhere else, so <td><option>x reads <td><select><option>x</option></select></td>, where a browser keeps the bare option. That is the free wrapper's normalisation, applied consistently; it was applied under div and every name before select already. The probe's fidelity rows are this, under u, strong, th and custom elements.
• A list item whose list the policy dropped, <ol><li>x with ol dropped and ul kept, comes out bare and gets a ul on the next pass, as on main; the change log says so. Judging it by the output as the option now is would be its own change.
• A container the policy renames into table structure, other than a template, still has the select fit judged by its logical name.
• A stray </select> closes the implied select, as it closes the one implied for <div><option> on main; a <style>, <script> or <textarea> inside the option's select holds no text, as CVE-2021-42575 requires and as it did under div. Both now apply under the containers that were past the bound.
• The balancer closes a table it pushes content out of, as it does for text, so <table><th><option>a</option></th><td>b with the cell dropped gives the select between two tables.
• <option><noscript><col> gives a bare <col /> on main, and still does; under Sanitizers.TABLES one probe input has it after a table the select is now pushed out of, where main left it inside.
• An unknown element between the dropped cell and the option is closed when the table is pushed out, as forwarded elements in a pushed-out table are, so text after its end tag joins the option: <table><th><foo><option>x</foo>y gives <select><option>xy</option></select> after the table. A form there takes the in-table rule of A form directly inside a table stays open as a container #484 and is popped, so the option runs on past </form> too. Neither is this item's; both are fixed points.
• <colgroup><foo><li>x implies the list inside the column group on the first pass and beside the table on the second, on main too; one probe input reaches it through a dropped template.
• A caption or column group under a dropped template (item 2) is a separate change.

Part of #492 (item 9)

🤖 Generated with Claude Code