Take shared-menu.php 1.2.0: the glyph is CSS-escaped, not esc_attr'd
Frozen by Nolimit SEO as 9012fe98, and it fixes something found here: the 1.1.0 glyph went through esc_attr() into a <style> element, where HTML entities are not decoded by the CSS parser. It was protecting by coincidence rather than by working, and Security found the case where the coincidence stops covering — a backslash, which esc_attr leaves alone, is a CSS escape and swallows the closing quote. No injection was possible on any input any of us could construct, so this is hardening rather than a hole, and the value comes from plugin code rather than from a request. The fix changes the alphabet instead of filtering the input: the glyph is emitted as CSS numeric escapes, so what reaches the stylesheet is nothing but backslashes, hex digits and spaces. cache-manager's statement of why that is the right shape is the one worth keeping — the value CANNOT CONTAIN A DELIMITER. "It escapes correctly" is a description, and a whitelist claims the same description while needing to be kept honest against inputs nobody has thought of yet. Verified here rather than accepted: - sha256 matches 9012fe98 before copying. - The "b8d9284b plus two full stops and a phpcs pair" claim is comment-only, measured with php -w over both: one sha256 for the stripped source. - The escaper itself run against seven inputs including a bare backslash, a quote, a CSS breakout, an HTML breakout and a NUL, asserting the output matches nothing outside [backslash, hex, space]. All seven clean. - Correct codepoints, and no mb_* calls, so the hand-written decoder holds on installs without mbstring — which is every install none of us test on. WPMGR_MENU_SHA updated in ci/plugin.conf. SEO's instruction said ci/static.sh, which is their layout, not this one — following it literally would have set a variable nothing reads, green on both sides while the check still asserted the old value. Found by searching for the OLD VALUE rather than the path: zero occurrences of any earlier frozen hash remain anywhere in ci/, tests/ or either workflow root, and the new one appears exactly once. Then proven consumed in both directions, which is the assertion that catches a copy nothing reads: changing the file by one byte moves "got", and changing the value in plugin.conf moves "expected". Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
This commit is contained in:
co-authored by
Claude Opus 5
parent
24efa2c8ce
commit
f908771f33
+9
-3
@@ -75,6 +75,12 @@ RELEASE_REPO=kw/nolimit-discounts-dist
|
||||
# The frozen hash is therefore a family-wide fact, not a local one, and taking
|
||||
# a new one is something all six do in the same window.
|
||||
#
|
||||
# 184cebdc = contract 1.1.0 (the ∞ glyph, and the header/count corrections
|
||||
# Nolimit SEO found). Previous frozen hash was f4862d5c = 1.0.0.
|
||||
WPMGR_MENU_SHA=184cebdcd86024d4e864c529bc68ead0f7d63656d9bb3dfe0e1fdf3203e364ae
|
||||
# 9012fe98 = contract 1.2.0. The glyph is now emitted as CSS numeric escapes
|
||||
# rather than through esc_attr(): HTML entities are not decoded by the CSS
|
||||
# parser, so esc_attr was protecting by coincidence — and a backslash, which it
|
||||
# leaves alone, IS a CSS escape and swallowed the closing quote. Changing the
|
||||
# alphabet beats filtering the input: what is emitted is nothing but
|
||||
# backslashes, hex digits and spaces, so there is nothing left to leave the
|
||||
# string with. Verified here against seven hostile inputs.
|
||||
# Earlier frozen hashes: 184cebdc = 1.1.0, f4862d5c = 1.0.0.
|
||||
WPMGR_MENU_SHA=9012fe98d867b86c993cee2f8d7b0b9fe5b89817fed4cf45346aa4949e2df307
|
||||
|
||||
@@ -69,7 +69,7 @@ if ( ! defined( 'ABSPATH' ) ) {
|
||||
}
|
||||
|
||||
if ( ! defined( 'WPMGR_MENU_VERSION' ) ) {
|
||||
define( 'WPMGR_MENU_VERSION', '1.1.0' );
|
||||
define( 'WPMGR_MENU_VERSION', '1.2.0' );
|
||||
}
|
||||
|
||||
if ( ! defined( 'WPMGR_MENU_SLUG' ) ) {
|
||||
@@ -168,11 +168,84 @@ if ( ! function_exists( 'wpmgr_ensure_menu' ) ) {
|
||||
return;
|
||||
}
|
||||
|
||||
// phpcs:disable WordPress.Security.EscapeOutput.OutputNotEscaped -- the slug is reduced to [a-z0-9_-] and the glyph to CSS numeric escapes; both are escaped for the context they are printed in, which is CSS rather than HTML.
|
||||
printf(
|
||||
'<style>#adminmenu .toplevel_page_%1$s .wp-menu-image::before{content:"%2$s";font-family:-apple-system,BlinkMacSystemFont,"Segoe UI",Roboto,sans-serif;font-size:20px;font-weight:400;line-height:1.4}</style>',
|
||||
esc_attr( WPMGR_MENU_SLUG ),
|
||||
esc_attr( $glyph )
|
||||
preg_replace( '/[^a-z0-9_-]/', '', strtolower( WPMGR_MENU_SLUG ) ),
|
||||
wpmgr_css_escape( $glyph )
|
||||
);
|
||||
// phpcs:enable WordPress.Security.EscapeOutput.OutputNotEscaped
|
||||
}
|
||||
|
||||
/**
|
||||
* A string that is safe inside a CSS string literal, whatever it was.
|
||||
*
|
||||
* 🔴 esc_attr() is the wrong tool here and it protected by accident.
|
||||
* Nolimit Discounts measured it: a glyph of `x"} body{display:none}` comes
|
||||
* out as `x"} …`, the CSS string never closes, and nothing breaks
|
||||
* out — but only because the entity happens to contain no quote. The CSS
|
||||
* parser does not decode HTML entities, so it was never escaping in this
|
||||
* context; it was a substitution that coincidentally removed the one
|
||||
* character that mattered. Two things follow from that: a glyph containing
|
||||
* a quote renders as the literal text `"` in the menu, and a single
|
||||
* backslash — which esc_attr() leaves alone, and which IS an escape to the
|
||||
* CSS parser — swallows the closing quote and the rest of the rule with
|
||||
* it. Measured, not reasoned: the string then runs on to the quote in
|
||||
* "Segoe UI" and the declaration block is corrupt.
|
||||
*
|
||||
* So do not filter the input; change its alphabet. Every character becomes
|
||||
* a CSS numeric escape, so the emitted value contains nothing but
|
||||
* backslashes, hex digits and spaces and cannot leave the string for any
|
||||
* input at all. No whitelist to keep honest, no character class to get
|
||||
* wrong, and an emoji still works.
|
||||
*
|
||||
* The decoder is written out rather than using mb_ord() because WordPress
|
||||
* supports installs without mbstring, and ord() on a multi-byte character
|
||||
* silently yields the first byte — which would render our own ∞ as
|
||||
* mojibake on exactly those installs and on no machine any of us test on.
|
||||
*
|
||||
* @param string $text Text from a filter, i.e. arbitrary.
|
||||
* @return string CSS escapes, safe between double quotes.
|
||||
*/
|
||||
function wpmgr_css_escape( $text ) {
|
||||
$out = '';
|
||||
$len = strlen( $text );
|
||||
|
||||
for ( $i = 0; $i < $len; $i++ ) {
|
||||
$byte = ord( $text[ $i ] );
|
||||
|
||||
if ( $byte < 0x80 ) {
|
||||
$code = $byte;
|
||||
$tail = 0;
|
||||
} elseif ( $byte >= 0xF0 ) {
|
||||
$code = $byte & 0x07;
|
||||
$tail = 3;
|
||||
} elseif ( $byte >= 0xE0 ) {
|
||||
$code = $byte & 0x0F;
|
||||
$tail = 2;
|
||||
} elseif ( $byte >= 0xC0 ) {
|
||||
$code = $byte & 0x1F;
|
||||
$tail = 1;
|
||||
} else {
|
||||
continue; // A stray continuation byte is not a character.
|
||||
}
|
||||
|
||||
while ( $tail > 0 && $i + 1 < $len && 0x80 === ( ord( $text[ $i + 1 ] ) & 0xC0 ) ) {
|
||||
++$i;
|
||||
$code = ( $code << 6 ) | ( ord( $text[ $i ] ) & 0x3F );
|
||||
--$tail;
|
||||
}
|
||||
|
||||
if ( $tail > 0 ) {
|
||||
continue; // Truncated sequence.
|
||||
}
|
||||
|
||||
// The trailing space terminates the escape, so a following hex
|
||||
// digit cannot be read as part of it.
|
||||
$out .= sprintf( '\\%X ', $code );
|
||||
}
|
||||
|
||||
return $out;
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
Reference in New Issue
Block a user