Skip to content

Commit 2e52bec

Browse files
pattonwebzclaude
andcommitted
test: assert excluded SVG elements are stripped as tags
Add regression vectors for the allow-list exclusions that carry a security or external-fetch rationale - raster <image>, the <filter> pipeline and its primitives (<feGaussianBlur>, <feImage>), and SMIL <animate> - asserting the tag names themselves are gone, not just their attributes. Also strengthens the animate case to check the whole tag is removed rather than only its onbegin handler. Addresses CodeRabbit test-coverage feedback on PR #1833. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 86ce4ae commit 2e52bec

1 file changed

Lines changed: 29 additions & 0 deletions

File tree

tests/phpunit/helper-functions/SanitizeScannedHtmlTest.php

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ public function malicious_svg_data() {
3636
'javascript: xlink' => [ '<svg><use xlink:href="javascript:alert(1)" /></svg>', 'javascript:' ],
3737
'javascript: href' => [ '<svg><use href="javascript:alert(1)" /></svg>', 'javascript:' ],
3838
'onbegin animate' => [ '<svg><animate onbegin="alert(1)" attributeName="x" /></svg>', 'onbegin' ],
39+
'animate element' => [ '<svg><animate onbegin="alert(1)" attributeName="x" /></svg>', '<animate' ],
3940
'onmouseover handler' => [ '<svg onmouseover="alert(1)"><rect width="10" height="10" /></svg>', 'onmouseover' ],
4041
];
4142
}
@@ -134,6 +135,34 @@ public function test_strips_svg_elements_outside_the_allow_list() {
134135
$this->assertStringNotContainsString( '<textpath', $sanitized );
135136
}
136137

138+
/**
139+
* Tests that non-rendering / scripting-adjacent SVG elements the allow-list
140+
* deliberately omits (raster <image>, the <filter> pipeline and its
141+
* primitives, and SMIL <animate>) are stripped as tags. These are called
142+
* out separately from the structural exclusions above because they are the
143+
* ones with a security or external-fetch rationale, not just "unused."
144+
*/
145+
public function test_strips_security_sensitive_svg_elements() {
146+
$svg = '<svg viewBox="0 0 10 10">'
147+
. '<image href="https://evil.example/x.svg" width="10" height="10" />'
148+
. '<filter id="f"><feGaussianBlur stdDeviation="1" /><feImage href="https://evil.example/y" /></filter>'
149+
. '<animate attributeName="x" from="0" to="10" />'
150+
. '<rect width="10" height="10" fill="#000" />'
151+
. '</svg>';
152+
153+
$sanitized = edac_sanitize_scanned_html( $svg );
154+
155+
$this->assertStringNotContainsStringIgnoringCase( '<image', $sanitized );
156+
$this->assertStringNotContainsStringIgnoringCase( '<filter', $sanitized );
157+
$this->assertStringNotContainsStringIgnoringCase( '<feGaussianBlur', $sanitized );
158+
$this->assertStringNotContainsStringIgnoringCase( '<feImage', $sanitized );
159+
$this->assertStringNotContainsStringIgnoringCase( '<animate', $sanitized );
160+
161+
// The benign sibling shape must still survive - excluding the above
162+
// should not nuke the rest of the graphic.
163+
$this->assertStringContainsString( '<rect', $sanitized );
164+
}
165+
137166
/**
138167
* Tests that a local same-document fragment reference (the common,
139168
* legitimate icon-sprite pattern) is preserved on both href and

0 commit comments

Comments
 (0)