Sanitize scanned object HTML at save time

admin/class-insert-rule-data.php

@@ -65,7 +65,11 @@ public function insert( object $post, string $rule, string $ruletype, string $ru
 			'xpath'             => $selectors['xpath'][0] ?? null,
 			'rule'              => $rule,
 			'ruletype'          => $ruletype,
-			'object'            => esc_attr( $rule_obj ),
+			// Sanitize before esc_attr(): the scanned object HTML/SVG is
+			// untrusted (submitted by any user who can edit the post, via
+			// the JS scan-results REST route) and is later html_entity_decode()d
+			// and re-parsed for display - see edac_sanitize_scanned_html().
+			'object'            => esc_attr( edac_sanitize_scanned_html( $rule_obj ) ),
 			'recordcheck'       => 1,
 			'user'              => get_current_user_id(),
 			'ignre'             => 0,
@@ -159,7 +163,12 @@ public function insert( object $post, string $rule, string $ruletype, string $ru
 				'xpath'             => sanitize_text_field( $rule_data['xpath'] ?? '' ),
 				'rule'              => sanitize_text_field( $rule_data['rule'] ),
 				'ruletype'          => sanitize_text_field( $rule_data['ruletype'] ),
-				'object'            => esc_attr( $rule_data['object'] ),
+				// Re-sanitize after the filter: a callback may have replaced
+				// 'object' with raw markup, or unset it entirely (hence ?? '').
+				// edac_sanitize_scanned_html() decodes first, so re-running it on
+				// the already-escaped line-72 value is idempotent for real content
+				// and does not double-encode.
+				'object'            => esc_attr( edac_sanitize_scanned_html( $rule_data['object'] ?? '' ) ),
 				'recordcheck'       => absint( $rule_data['recordcheck'] ),
 				'user'              => absint( $rule_data['user'] ),
 				'ignre'             => absint( $rule_data['ignre'] ),

includes/helper-functions.php

@@ -788,6 +788,195 @@ function edac_parse_html_for_media( $html ) {
 	];
 }
 
+/**
+ * Allowed tags/attributes for edac_sanitize_scanned_html() - wp_kses_allowed_html( 'post' )
+ * plus the minimal SVG vocabulary a flagged icon/logo/decorative graphic
+ * actually uses: container, grouping, basic shapes, gradients, text, and the
+ * accessible-name elements (<title>/<desc>). Deliberately excludes <script>,
+ * <foreignObject>, <image>, SMIL animation, filter primitives, and
+ * rarely-used structural extras (<pattern>, <mask>, <marker>, <switch>,
+ * <textPath>) - none of which this plugin's real-world content needs. Note
+ * that <a> IS allowed, via the 'post' base list: wp_kses() has no namespace
+ * awareness, so that entry also matches <a> inside <svg> markup. That's safe
+ * because no on* attribute is ever allowed on anything and href values get
+ * core's bad-protocol validation.
+ *
+ * Every allowed SVG element shares one attribute set: wp_kses() only needs
+ * attribute names allow-listed, not semantically scoped per tag, and a
+ * geometry/paint attribute on a tag that ignores it is harmless. The only
+ * exception is href/xlink:href, which stays scoped to <use> (the icon-sprite
+ * pattern) and is protocol-validated - see edac_sanitize_scanned_html().
+ *
+ * @since x.x.x
+ *
+ * @return array<string, array<string, bool>>
+ */
+function edac_scanned_html_allowed_tags(): array {
+	$identity = [ 'id', 'class', 'style', 'transform', 'role', 'focusable' ];
+	$aria     = [ 'aria-hidden', 'aria-label', 'aria-labelledby', 'aria-describedby' ];
+	$root     = [ 'xmlns', 'xmlns:xlink', 'version', 'viewbox', 'preserveaspectratio' ];
+	$geometry = [ 'x', 'y', 'width', 'height', 'cx', 'cy', 'r', 'rx', 'ry', 'x1', 'y1', 'x2', 'y2', 'fx', 'fy', 'd', 'points', 'dx', 'dy' ];
+	$paint    = [ 'fill', 'fill-rule', 'fill-opacity', 'stroke', 'stroke-width', 'stroke-linecap', 'stroke-linejoin', 'stroke-dasharray', 'stroke-dashoffset', 'stroke-opacity', 'stroke-miterlimit', 'opacity', 'clip-path', 'clip-rule' ];
+	$gradient = [ 'gradientunits', 'gradienttransform', 'spreadmethod', 'offset', 'stop-color', 'stop-opacity' ];
+	$text     = [ 'text-anchor', 'font-family', 'font-size', 'font-weight', 'font-style' ];
+
+	$svg_attributes = array_fill_keys(
+		array_merge( $identity, $aria, $root, $geometry, $paint, $gradient, $text ),
+		true
+	);
+
+	$svg_elements = [
+		'svg',
+		'g',
+		'defs',
+		'symbol',
+		'use',
+		'path',
+		'rect',
+		'circle',
+		'ellipse',
+		'line',
+		'polyline',
+		'polygon',
+		'lineargradient',
+		'radialgradient',
+		'stop',
+		'clippath',
+		'text',
+		'tspan',
+		'title',
+		'desc',
+	];
+
+	$svg = array_fill_keys( $svg_elements, $svg_attributes );
+
+	// Local same-document fragment reference (#id) for the icon-sprite
+	// pattern. edac_sanitize_scanned_html() registers xlink:href with
+	// wp_kses_uri_attributes() for the duration of the call, so both get
+	// the same bad-protocol/URI validation core already applies to the
+	// plain 'href' attribute.
+	$svg['use']['href']       = true;
+	$svg['use']['xlink:href'] = true;
+
+	return array_merge( wp_kses_allowed_html( 'post' ), $svg );
+}
+
+/**
+ * SVG attribute names are case-sensitive per spec (e.g. viewBox,
+ * gradientTransform), but wp_kses() - built for case-insensitive HTML -
+ * lowercases every attribute name it outputs. Left alone, that silently
+ * breaks otherwise-safe SVGs (a lowercased viewbox is simply ignored by
+ * browsers). Keyed by the lowercased name wp_kses() produces. Every SVG
+ * attribute with a case-sensitive canonical spelling that appears (lowercased)
+ * in edac_scanned_html_allowed_tags() must have an entry here.
+ *
+ * @since x.x.x
+ *
+ * @return array<string, string>
+ */
+function edac_svg_case_sensitive_attributes(): array {
+	return [
+		'viewbox'             => 'viewBox',
+		'preserveaspectratio' => 'preserveAspectRatio',
+		'gradientunits'       => 'gradientUnits',
+		'gradienttransform'   => 'gradientTransform',
+		'spreadmethod'        => 'spreadMethod',
+	];
+}
+
+/**
+ * Restore case-sensitive SVG attribute names that wp_kses() lowercased.
+ *
+ * @since x.x.x
+ *
+ * @param string $sanitized_html HTML already run through wp_kses().
+ * @return string
+ */
+function edac_restore_svg_attribute_case( string $sanitized_html ): string {
+	if ( false === stripos( $sanitized_html, '<svg' ) ) {
+		// Cheap early exit - none of these attributes mean anything outside SVG.
+		return $sanitized_html;
+	}
+
+	$replacements = edac_svg_case_sensitive_attributes();
+	$names        = implode( '|', array_map( 'preg_quote', array_keys( $replacements ) ) );
+
+	// Restore every case-sensitive attribute in one outer pass over the tags:
+	// match each tag, then fix all matching attribute names inside it. A flat
+	// preg_replace_callback with alternation cannot do this because it consumes
+	// the opening "<", leaving later attributes on the same tag unanchored.
+	$restored = preg_replace_callback(
+		'/<[^>]+>/',
+		static function ( array $tag ) use ( $replacements, $names ): string {
+			return preg_replace_callback(
+				'/(\s)(' . $names . ')(\s*=)/i',
+				static function ( array $attr ) use ( $replacements ): string {
+					return $attr[1] . ( $replacements[ strtolower( $attr[2] ) ] ?? $attr[2] ) . $attr[3];
+				},
+				$tag[0]
+			);
+		},
+		$sanitized_html
+	);
+
+	return null === $restored ? $sanitized_html : $restored;
+}
+
+/**
+ * Sanitize an HTML/SVG snippet before it's stored as an issue's "object"
+ * (the scanned element's outerHTML) - strips constructs that could execute
+ * as script (script tags, on* event handler attributes, <foreignObject>)
+ * while leaving ordinary post-content HTML and non-scripting SVG markup
+ * (shapes, gradients, text) unmodified.
+ *
+ * The stored value is later html_entity_decode()d and re-parsed for display
+ * (see edac_parse_html_for_media() and the Frontend_Highlight AJAX handler).
+ * That downstream decode is why we decode here first: entity-encoded markup
+ * such as `&lt;img src=x onerror=alert(1)&gt;` is inert *text* to wp_kses()
+ * (nothing to strip), but the display-side decode would revive it into a live
+ * tag - a sanitizer bypass. Decoding to a fixed point up front (so multiply
+ * -encoded payloads like `&amp;lt;...` collapse too) means wp_kses() sees the
+ * exact markup a browser will ultimately parse, and its decision is the one
+ * that sticks.
+ *
+ * @since x.x.x
+ *
+ * @param mixed $html Raw HTML snippet - expected to be a string.
+ * @return string Sanitized HTML, or '' if given anything other than a string.
+ */
+function edac_sanitize_scanned_html( $html ): string {
+	if ( ! is_string( $html ) || '' === $html ) {
+		return '';
+	}
+
+	// Fully decode entities before sanitizing - see docblock. html_entity_decode()
+	// only ever contracts (entities -> single chars), so this cannot expand the
+	// input; the guard just bounds pathological deep nesting.
+	$previous     = null;
+	$decode_guard = 0;
+	while ( $html !== $previous && $decode_guard < 10 ) {
+		$previous = $html;
+		$html     = html_entity_decode( $html, ENT_QUOTES | ENT_HTML5 );
+		++$decode_guard;
+	}
+
+	// xlink:href isn't in core's wp_kses_uri_attributes() list (that list
+	// predates SVG/XLink awareness), so without this filter a value like
+	// xlink:href="javascript:alert(1)" would pass through unchecked even
+	// though the identical case for the plain 'href' attribute is already
+	// protocol-validated by core. Scoped to this call only.
+	$add_xlink_href = static function ( array $uri_attributes ): array {
+		$uri_attributes[] = 'xlink:href';
+		return $uri_attributes;
+	};
+
+	add_filter( 'wp_kses_uri_attributes', $add_xlink_href );
+	$sanitized = wp_kses( $html, edac_scanned_html_allowed_tags() );
+	remove_filter( 'wp_kses_uri_attributes', $add_xlink_href );
+
+	return edac_restore_svg_attribute_case( $sanitized );
+}
+
 /**
  * Remove corrected posts
  *