diff --git a/ci/plugin.conf b/ci/plugin.conf index 0a0ce70..6e7dc0d 100644 --- a/ci/plugin.conf +++ b/ci/plugin.conf @@ -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 diff --git a/includes/shared-menu.php b/includes/shared-menu.php index 86dd81b..bd73360 100644 --- a/includes/shared-menu.php +++ b/includes/shared-menu.php @@ -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( '', - 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; } /**