You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Fix stored HTML injection in comment markdown (GO-39) - #2944
Markdown in a saved comment or activity field could introduce HTML attributes when formatComment interpolated unescaped image/link captures into the comment and activity templates. This addresses weekly security scan finding GO-39, originally traced at faa6318de8d27126222630335b2a5d893908ce1c (v1.83.6).
Escape plain text, labels, image alt text, and URL attributes inside the shared formatter, preserving one existing level of WordPress HTML entity encoding. Parse image, mention, and link markdown in one pass and protect generated markup from later formatting replacements. Only HTTP/HTTPS destinations are emitted, while record-relative markdown links still resolve through the site's record URL.
Preserve mentions (including RTL names), emphasis, lists, external links, internal record links, images with HTTP/HTTPS URLs, and automatic links. Raw HTML now displays literally; unsupported schemes, protocol-relative destinations, and relative image URLs remain escaped markdown. Automatic links also work beside images. Keep raw activity notes unchanged across renders and let the shared formatter escape grouped activity comments exactly once, after date substitution.
Validation:
npm run test:js: 13 passing regression tests, including both activity paths, repeated rendering, quote injection, unsafe schemes, entities, and formatting compatibility. The suite fails on unpatched develop.
Offline Chromium DOM parsing: 351 representative/adversarial inputs produced no unexpected tags, attributes, unresolved placeholders, or non-HTTP/HTTPS destinations. This check used an inert template, without a live WordPress session.
php -l: all 973 tracked PHP/inc/lib files passed.
vendor/bin/phpcs: passed.
ESLint, Prettier, Node syntax checks for the changed JavaScript, and git diff --check: passed.
Added the JavaScript helper tests to CI. No generated bundle update is needed: these scripts are enqueued directly.
Live verification: after the user supplied the correct local URL, tested the patch through an authenticated browser session on https://multisite.lwp. A disposable contact exercised saved comments, text-field activity, HTTP/HTTPS image rendering, record and external links with query parameters, multilingual mentions, emphasis, lists, ampersands, and repeated rendering through tab toggles. Harmless quote-injection markers in both comments and text-field activity did not execute their page-title change; a JavaScript-scheme markdown link remained literal text. This check caught and corrected double encoding of WordPress's existing & entities. The test contact was renamed as completed and archived, and the three local source files were restored after testing; the PR remains isolated in its worktree.
The original finding was a static trace while the local site was unavailable. A separate Multiplier-to-admin session reproduction, grouped activity log/personal highlights UI verification, and privilege-changing REST actions were not performed. The automated suite covers both activity rendering paths and repeat rendering. Review the intentional raw-HTML and URL-scheme restrictions above before merging.
Based on develop, verified as the repository's default branch with gh repo view.
Comment edit flow regresses for <, > and other entities (dt-assets/js/comments.js, the .open-edit-comment handler at ~line 327, plus the removed block at ~line 611)
The PR removes the line that set comment.comment_content to $("<div>").text(comment.comment_content).text(). That line also decoded all HTML entities in the stored comment_content (<, >, ’, and so on) as a side effect. The edit handler reads comment.comment_content and passes it through unescapeHtml(), which only handles &, ", ' and '. Its < and > replacements are commented out because the earlier decode covered them.
Now comment_content keeps its raw entities, so editing a comment that contains <, > or numeric entities fills the textarea with literal < or >. Saving re-encodes those to &lt;, so the comment is double-encoded and corrupted.
Trigger: post a comment containing a < b, click edit, save. Rendering is fine. The edit textarea and the saved value are wrong.
Fix: decode the stored value for the textarea, for example via a detached element, or extend unescapeHtml to handle <, > and numeric entities. Add a test for the edit path.
Notes (no action needed)
The placeholder scheme in formatComment looks sound. NULs are stripped from the input, and the list-handling early return is unreachable.
The activity-log.js and comments.js rendering changes escape exactly once.
Summary: The XSS hardening looks solid and well tested, but the edit-comment regression should be fixed before merge.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Markdown in a saved comment or activity field could introduce HTML attributes when
formatCommentinterpolated unescaped image/link captures into the comment and activity templates. This addresses weekly security scan finding GO-39, originally traced atfaa6318de8d27126222630335b2a5d893908ce1c(v1.83.6).Escape plain text, labels, image alt text, and URL attributes inside the shared formatter, preserving one existing level of WordPress HTML entity encoding. Parse image, mention, and link markdown in one pass and protect generated markup from later formatting replacements. Only HTTP/HTTPS destinations are emitted, while record-relative markdown links still resolve through the site's record URL.
Preserve mentions (including RTL names), emphasis, lists, external links, internal record links, images with HTTP/HTTPS URLs, and automatic links. Raw HTML now displays literally; unsupported schemes, protocol-relative destinations, and relative image URLs remain escaped markdown. Automatic links also work beside images. Keep raw activity notes unchanged across renders and let the shared formatter escape grouped activity comments exactly once, after date substitution.
Validation:
npm run test:js: 13 passing regression tests, including both activity paths, repeated rendering, quote injection, unsafe schemes, entities, and formatting compatibility. The suite fails on unpatcheddevelop.php -l: all 973 tracked PHP/inc/lib files passed.vendor/bin/phpcs: passed.git diff --check: passed.Live verification: after the user supplied the correct local URL, tested the patch through an authenticated browser session on
https://multisite.lwp. A disposable contact exercised saved comments, text-field activity, HTTP/HTTPS image rendering, record and external links with query parameters, multilingual mentions, emphasis, lists, ampersands, and repeated rendering through tab toggles. Harmless quote-injection markers in both comments and text-field activity did not execute their page-title change; a JavaScript-scheme markdown link remained literal text. This check caught and corrected double encoding of WordPress's existing&entities. The test contact was renamed as completed and archived, and the three local source files were restored after testing; the PR remains isolated in its worktree.The original finding was a static trace while the local site was unavailable. A separate Multiplier-to-admin session reproduction, grouped activity log/personal highlights UI verification, and privilege-changing REST actions were not performed. The automated suite covers both activity rendering paths and repeat rendering. Review the intentional raw-HTML and URL-scheme restrictions above before merging.
Based on
develop, verified as the repository's default branch withgh repo view.