Skip to content

Commit

Permalink
Added extra HTML filtering of dangerous content
Browse files Browse the repository at this point in the history 
In particular, That around the casing of dangerous values within
attributes. This uses some xpath translation to handle different casing
in contains searching.
  • Loading branch information
@ssddanbrown
ssddanbrown committed Sep 2, 2021
1 parent 7028025 commit 5e6092a
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 6 deletions.
19 changes: 15 additions & 4 deletions app/Util/HtmlContentFilter.php
View file
Expand Up @@ -28,19 +28,19 @@ public static function removeScripts(string $html): string
static::removeNodes($scriptElems);

// Remove clickable links to JavaScript URI
$badLinks = $xPath->query('//*[contains(@href, \'javascript:\')]');
$badLinks = $xPath->query('//*[' . static::xpathContains('@href', 'javascript:') . ']');
static::removeNodes($badLinks);

// Remove forms with calls to JavaScript URI
$badForms = $xPath->query('//*[contains(@action, \'javascript:\')] | //*[contains(@formaction, \'javascript:\')]');
$badForms = $xPath->query('//*[' . static::xpathContains('@action', 'javascript:') . '] | //*[' . static::xpathContains('@formaction', 'javascript:') . ']');
static::removeNodes($badForms);

// Remove meta tag to prevent external redirects
$metaTags = $xPath->query('//meta[contains(@content, \'url\')]');
$metaTags = $xPath->query('//meta[' . static::xpathContains('@content', 'url') . ']');
static::removeNodes($metaTags);

// Remove data or JavaScript iFrames
$badIframes = $xPath->query('//*[contains(@src, \'data:\')] | //*[contains(@src, \'javascript:\')] | //*[@srcdoc]');
$badIframes = $xPath->query('//*[' . static::xpathContains('@src', 'data:') . '] | //*[' . static::xpathContains('@src', 'javascript:') . '] | //*[@srcdoc]');
static::removeNodes($badIframes);

// Remove 'on*' attributes
Expand All @@ -60,6 +60,17 @@ public static function removeScripts(string $html): string
return $html;
}

/**
* Create a xpath contains statement with a translation automatically built within
* to affectively search in a cases-insensitive manner.
*/
protected static function xpathContains(string $property, string $value): string
{
$value = strtolower($value);
$upperVal = strtoupper($value);
return 'contains(translate(' . $property . ', \'' . $upperVal . '\', \'' . $value . '\'), \'' . $value . '\')';
}

/**
* Removed all of the given DOMNodes.
*/
Expand Down
25 changes: 23 additions & 2 deletions tests/Entity/PageContentTest.php
View file
Expand Up @@ -135,14 +135,26 @@ public function test_more_complex_content_script_escaping_scenarios()
}
}

public function test_iframe_js_and_base64_urls_are_removed()
public function test_js_and_base64_src_urls_are_removed()
{
$checks = [
'<iframe src="javascript:alert(document.cookie)"></iframe>',
'<iframe src="JavAScRipT:alert(document.cookie)"></iframe>',
'<iframe src="JavAScRipT:alert(document.cookie)"></iframe>',
'<iframe SRC=" javascript: alert(document.cookie)"></iframe>',
'<iframe src="data:text/html;base64,PHNjcmlwdD5hbGVydCgnaGVsbG8nKTwvc2NyaXB0Pg==" frameborder="0"></iframe>',
'<iframe src="DaTa:text/html;base64,PHNjcmlwdD5hbGVydCgnaGVsbG8nKTwvc2NyaXB0Pg==" frameborder="0"></iframe>',
'<iframe src=" data:text/html;base64,PHNjcmlwdD5hbGVydCgnaGVsbG8nKTwvc2NyaXB0Pg==" frameborder="0"></iframe>',
'<img src="javascript:alert(document.cookie)"/>',
'<img src="JavAScRipT:alert(document.cookie)"/>',
'<img src="JavAScRipT:alert(document.cookie)"/>',
'<img SRC=" javascript: alert(document.cookie)"/>',
'<img src="data:text/html;base64,PHNjcmlwdD5hbGVydCgnaGVsbG8nKTwvc2NyaXB0Pg=="/>',
'<img src="DaTa:text/html;base64,PHNjcmlwdD5hbGVydCgnaGVsbG8nKTwvc2NyaXB0Pg=="/>',
'<img src=" data:text/html;base64,PHNjcmlwdD5hbGVydCgnaGVsbG8nKTwvc2NyaXB0Pg=="/>',
'<iframe srcdoc="<script>window.alert(document.cookie)</script>"></iframe>',
'<iframe SRCdoc="<script>window.alert(document.cookie)</script>"></iframe>',
'<IMG SRC=`javascript:alert("RSnake says, \'XSS\'")`>',
];

$this->asEditor();
Expand All @@ -155,6 +167,7 @@ public function test_iframe_js_and_base64_urls_are_removed()
$pageView = $this->get($page->getUrl());
$pageView->assertStatus(200);
$pageView->assertElementNotContains('.page-content', '<iframe>');
$pageView->assertElementNotContains('.page-content', '<img');
$pageView->assertElementNotContains('.page-content', '</iframe>');
$pageView->assertElementNotContains('.page-content', 'src=');
$pageView->assertElementNotContains('.page-content', 'javascript:');
Expand All @@ -168,6 +181,8 @@ public function test_javascript_uri_links_are_removed()
$checks = [
'<a id="xss" href="javascript:alert(document.cookie)>Click me</a>',
'<a id="xss" href="javascript: alert(document.cookie)>Click me</a>',
'<a id="xss" href="JaVaScRiPt: alert(document.cookie)>Click me</a>',
'<a id="xss" href=" JaVaScRiPt: alert(document.cookie)>Click me</a>',
];

$this->asEditor();
Expand All @@ -179,7 +194,7 @@ public function test_javascript_uri_links_are_removed()

$pageView = $this->get($page->getUrl());
$pageView->assertStatus(200);
$pageView->assertElementNotContains('.page-content', '<a id="xss">');
$pageView->assertElementNotContains('.page-content', '<a id="xss"');
$pageView->assertElementNotContains('.page-content', 'href=javascript:');
}
}
Expand All @@ -188,8 +203,10 @@ public function test_form_actions_with_javascript_are_removed()
{
$checks = [
'<form><input id="xss" type=submit formaction=javascript:alert(document.domain) value=Submit><input></form>',
'<form ><button id="xss" formaction="JaVaScRiPt:alert(document.domain)">Click me</button></form>',
'<form ><button id="xss" formaction=javascript:alert(document.domain)>Click me</button></form>',
'<form id="xss" action=javascript:alert(document.domain)><input type=submit value=Submit></form>',
'<form id="xss" action="JaVaScRiPt:alert(document.domain)"><input type=submit value=Submit></form>',
];

$this->asEditor();
Expand All @@ -213,6 +230,8 @@ public function test_metadata_redirects_are_removed()
{
$checks = [
'<meta http-equiv="refresh" content="0; url=//external_url">',
'<meta http-equiv="refresh" ConTeNt="0; url=//external_url">',
'<meta http-equiv="refresh" content="0; UrL=//external_url">',
];

$this->asEditor();
Expand Down Expand Up @@ -249,11 +268,13 @@ public function test_more_complex_inline_on_attributes_escaping_scenarios()
{
$checks = [
'<p onclick="console.log(\'test\')">Hello</p>',
'<p OnCliCk="console.log(\'test\')">Hello</p>',
'<div>Lorem ipsum dolor sit amet.</div><p onclick="console.log(\'test\')">Hello</p>',
'<div>Lorem ipsum dolor sit amet.<p onclick="console.log(\'test\')">Hello</p></div>',
'<div><div><div><div>Lorem ipsum dolor sit amet.<p onclick="console.log(\'test\')">Hello</p></div></div></div></div>',
'<div onclick="console.log(\'test\')">Lorem ipsum dolor sit amet.</div><p onclick="console.log(\'test\')">Hello</p><div></div>',
'<a a="<img src=1 onerror=\'alert(1)\'> ',
'\<a onclick="alert(document.cookie)"\>xss link\</a\>',
];

$this->asEditor();
Expand Down

0 comments on commit 5e6092a

Please sign in to comment.