fix: use-after-free in CleanNode when TidyGDocClean discards STYLE element

[SongTonyLi]

Summary

Fixes #1175.

CleanNode() in gdoc.c traverses the DOM tree, saving next = child->next then conditionally pruning child. The STYLE and empty-P checks were separate if statements (not else if), so when the STYLE branch calls DiscardElement() to free child, control falls through to the P check which dereferences the already-freed child. This is a heap-use-after-free (CWE-416) reachable via crafted HTML with TidyGDocClean enabled.

Root cause

Call chain: tidyCleanAndRepair() -> tidyDocCleanAndRepair() -> prvTidyCleanGoogleDocument() -> CleanNode().

The loop at gdoc.c:106 saves next = child->next, then at lines 110-111 the STYLE branch calls TY_(DiscardElement)(doc, child) which frees child through prvTidyFreeNode(). The immediately following if (nodeIsP(child) && !child->content) at line 112 is a separate condition (not else if), so it dereferences the freed pointer.

Forward instrumentation confirmed: DiscardElement freed element at 0x50b0000001a0, and ASan reported invalid read at 0x50b0000001d8, which is 56 bytes inside the freed 112-byte region [0x50b0000001a0, 0x50b000000210).

Fix

Two changes in CleanNode() at src/gdoc.c:

1. STYLE branch captures return value: next = TY_(DiscardElement)(doc, child) so the traversal cursor advances past the freed node.
2. Branches made mutually exclusive: the empty-P check becomes else if, preventing re-evaluation of freed child in the same iteration.

Why this approach

I looked at three possible fixes:

• Traversal-state repair at the mutation site (went with this one): makes the STYLE/P branches exclusive and captures DiscardElement's return into next, aligning with the existing pattern in this function where DiscardContainer already updates next via out-param.
• Guard at crash condition (e.g., NULL check before the P test): doesn't actually fix the bug since it relies on inspecting a stale pointer, which is undefined behavior in C. It's crash suppression, not a root-cause fix.
• Broad defensive hardening in DiscardElement/FreeNode: too much scope and regression risk for a localized traversal bug.

Verification

Tested with ASan-instrumented build (-fsanitize=address, detect_leaks=1):

• Before: SUMMARY: AddressSanitizer: heap-use-after-free /src/tidy-html5/src/gdoc.c:112:21 in CleanNode
• After: no heap-use-after-free; clean exit

Note: ASan still reports 192 bytes leaked in 8 allocations, but these are pre-existing leaks in prvTidynewStack (present with and without the patch) and unrelated to this fix.