Skip to content

Escape values interpolated into HTML attributes - #768

Closed
obenland wants to merge 1 commit into
WordPress:trunkfrom
obenland:fix/attribute-escaping-sweep
Closed

Escape values interpolated into HTML attributes#768
obenland wants to merge 1 commit into
WordPress:trunkfrom
obenland:fix/attribute-escaping-sweep

Conversation

@obenland

@obenland obenland commented Aug 6, 2026

Copy link
Copy Markdown
Member

A sweep of templates that build HTML attributes by interpolation without an escaper, or with one that does not match the context. Each change is a straightforward output-escaping correction.

Rosetta

front-page.php's blog list applied esc_attr__() to the printf() format string — a hardcoded developer constant with nothing to escape — while the post title passed as the argument was interpolated raw. Reworked to escape the composed string.

Showcase

  • single.php / 404.php: the gallery loop interpolated the two halves of the image custom field into single-quoted href and title attributes with no escaper. Now esc_url() / esc_attr(). The ?w=155&h=155 cache-buster was also being concatenated onto a URL that may already carry a query string, so it now goes through add_query_arg().
  • functions.php: site_screenshot_tag() escaped alt but not src, srcset, width, height or class. site_screenshot_src() echoed its URL raw.
  • js/fancyzoom.js: the caption is a plain string, so it is now written with textContent rather than innerHTML. prepZooms() also scanned every anchor in the document; it is now scoped to the theme's own .gallery container, so unrelated links are not given zoom handlers.

Support forums

bbPress registers bbp_rel_nofollow on bbp_get_topic_content and bbp_get_reply_content at priority 60. It parses attributes with shortcode_parse_atts(), which runs stripcslashes() over every value, and then re-emits them with no escaping. class-hooks.php already removes this filter from the author-link hooks, and adds its own add_rel_ugc() at priority 80 — which builds attributes via core's wp_rel_callback() and escapes them. Removing the priority-60 filter from the two content hooks therefore loses no behaviour.

This one is a mitigation, not the root fix: the defect is in bbp_rel_nofollow_callback() in bbPress itself, which is unfixed upstream and affects every bbPress site. That needs a separate fix there — wp_kses_hair() in place of shortcode_parse_atts(), plus esc_attr() on re-emission. Note an escaper alone is not sufficient upstream, since the decode has already happened by that point. Removing the filter here is worth doing regardless, because on WordPress.org it is pure redundancy.

i18n teams / support profiles

Slack display names are synced from an external source and were printed raw into markup on support profiles and locale team pages. Escaped at the three wp-i18n-teams sinks and at the wporg-support-2024 profile sink.

The getters are deliberately left returning the raw value. sanitize_text_field() there would be the wrong tool — it is a sanitiser in a read position, and it is lossy for legitimate names, since it strips percent-encoded sequences (50%20off50off). If a defence-in-depth layer is wanted it belongs at import, where slack_users.profiledata is written.

Notes

  • wporg-learn-2024's search-results-context block has the same class of issue — esc_attr() used for an element-name position. It is mirrored here from WordPress/Learn, so the fix goes there instead and will arrive on the next sync: Search Results Context: Allowlist the tagName attribute Learn#3586.
  • wporg-showcase appears superseded by wporg-showcase-2022 and has had no commits since June 2022; the fixes are included since the theme is still in the tree, but removing the theme would be the better close. That would also drop fancyzoom.js, which carries a licence requiring a per-domain fee for commercial use and forbidding sale of derived works.
  • Linted with ./vendor/bin/phpcs; no new violations on the changed lines. Pre-existing violations in these files are left alone.

🤖 Generated with Claude Code

Copilot AI lite review requested due to automatic review settings August 6, 2026 19:57
@obenland
obenland force-pushed the fix/attribute-escaping-sweep branch from 8a35e01 to fa65bfd Compare August 6, 2026 20:01
…ated into HTML attributes.

Several templates build attributes by interpolation without an escaper, or
apply one that does not match the context:

- Rosetta's front-page blog list escaped the printf() format string rather
  than the post title passed as its argument.
- The Showcase gallery and screenshot tags interpolated post meta into
  single-quoted attributes with no escaper, and FancyZoom wrote an anchor's
  title into innerHTML while binding to every anchor on the page.
- The support forums kept bbPress's priority-60 rel=nofollow rewriter on
  topic and reply content; it re-emits attributes unescaped after
  shortcode_parse_atts() has run stripcslashes() over them. add_rel_ugc()
  at priority 80 already adds nofollow safely.
- Slack display names synced from an external source were printed raw on
  support profiles and locale team pages.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@obenland
obenland force-pushed the fix/attribute-escaping-sweep branch from fa65bfd to 6e5e604 Compare August 6, 2026 20:15

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@github-actions

github-actions Bot commented Aug 6, 2026

Copy link
Copy Markdown

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Core Committers: Use this line as a base for the props when committing in SVN:

Props obenland.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants