Opened 19 months ago
Last modified 8 weeks ago
#62545 new defect (bug)
Adminbar title not escaped
| Reported by: |
|
Owned by: | |
|---|---|---|---|
| Milestone: | Awaiting Review | Priority: | normal |
| Severity: | minor | Version: | |
| Component: | Toolbar | Keywords: | 2nd-opinion has-patch has-unit-tests |
| Focuses: | Cc: |
Description
class-wp-admin-bar.php
<?php echo ">{$arrow}{$node->title}";
The title isn't getting escaped, there should be an esc_html() there?
Attachments (2)
Change History (11)
This ticket was mentioned in PR #7876 on WordPress/wordpress-develop by lgadzhev.
19 months ago
#2
- Keywords has-patch added
@siliconforks commented on PR #7876:
19 months ago
#3
#4
@
19 months ago
- Component changed from Security to Toolbar
- Keywords close 2nd-opinion added; has-patch removed
It should not use esc_html() because that would break almost all links with an icon or image, including links—and the search form—from Core.
I'll leave the ticket open in case there is a better way to sanitize the node titles (or to skip creating a node if it finds something that does not belong).
@
19 months ago
Patch to use wp_kses_post to escape the admin bar title instead of using esc_url or not escaping at all.
#5
@
19 months ago
@sabernhardt, If escaping is required in this context, we can utilize wp_kses_post(). This function allows safe HTML to pass through while preventing potentially harmful elements, avoiding the side effects illustrated in the screenshot.
#6
@
19 months ago
The wp_kses_post() function does not fit either. It would change & to &, but it would also break the search node (the screenshot shows the front end without a search icon). The default KSES post array does not allow form or input elements, and the filter could remove other elements that plugins rely on.
#7
@
15 months ago
- Keywords close removed
The title probably could normalize HTML entities:
wp_kses_normalize_entities( $node->title )
That is not escaping, but the function is part of esc_html().
@jonsurrell commented on PR #7876:
10 months ago
#8
The GitHub user that created this PR has been deleted. Closing the PR.
This ticket was mentioned in PR #11612 on WordPress/wordpress-develop by @dhrupo.
8 weeks ago
#9
- Keywords has-patch has-unit-tests added
## Summary
Normalize HTML entities when rendering admin bar node titles without escaping valid markup used by core and plugins.
## Problem
WP_Admin_Bar::_render_item() outputs $node->title directly. This means text like This & that is rendered with a literal & instead of a normalized &, while broad escaping approaches would break existing admin bar titles that intentionally include HTML.
## Solution
Use wp_kses_normalize_entities() when rendering the admin bar title. This normalizes entities in text content while preserving valid embedded markup.
## Testing
Added a PHPUnit regression test covering an admin bar title that includes both HTML markup and an ampersand.
Verified in the configured local wordpress-develop environment with:
node ./tools/local-env/scripts/docker.js exec --user wp_php php ./vendor/bin/phpunit --filter test_admin_bar_normalizes_title_entities_without_escaping_html tests/phpunit/tests/adminbar.phpnode ./tools/local-env/scripts/docker.js exec --user wp_php php ./vendor/bin/phpunit tests/phpunit/tests/adminbar.php
e.g.
The & will not be & but literal