BUG HTMLText whitelist considers text nodes

Minor improvement to #2853.
If a list of whitelisted elements are specified, text nodes no longer evade the whitelist
This commit is contained in:
Damian Mooyman 2014-04-09 14:48:02 +12:00
parent ee487e28f1
commit 91034d1341
2 changed files with 33 additions and 10 deletions

View File

@ -55,8 +55,9 @@ class HTMLText extends Text {
* - whitelist: If provided, a comma-separated list of elements that will be allowed to be stored
* (be careful on relying on this for XSS protection - some seemingly-safe elements allow
* attributes that can be exploited, for instance <img onload="exploiting_code();" src="..." />)
*
* @return void
* Text nodes outside of HTML tags are filtered out by default, but may be included by adding
* the text() directive. E.g. 'link,meta,text()' will allow only <link /> <meta /> and text at
* the root level.
*/
public function setOptions(array $options = array()) {
parent::setOptions($options);
@ -184,20 +185,36 @@ class HTMLText extends Text {
}
public function prepValueForDB($value) {
return parent::prepValueForDB($this->whitelistContent($value));
}
/**
* Filter the given $value string through the whitelist filter
*
* @param string $value Input html content
* @return string Value with all non-whitelisted content stripped (if applicable)
*/
public function whitelistContent($value) {
if($this->whitelist) {
$dom = Injector::inst()->create('HTMLValue', $value);
$query = array();
foreach ($this->whitelist as $tag) $query[] = 'not(self::'.$tag.')';
$textFilter = ' | //body/text()';
foreach ($this->whitelist as $tag) {
if($tag === 'text()') {
$textFilter = ''; // Disable text filter if allowed
} else {
$query[] = 'not(self::'.$tag.')';
}
}
foreach($dom->query('//body//*['.implode(' and ', $query).']') as $el) {
foreach($dom->query('//body//*['.implode(' and ', $query).']'.$textFilter) as $el) {
if ($el->parentNode) $el->parentNode->removeChild($el);
}
$value = $dom->getContent();
}
return parent::prepValueForDB($value);
return $value;
}
/**

View File

@ -179,11 +179,17 @@ class HTMLTextTest extends SapphireTest {
function testWhitelist() {
$textObj = new HTMLText('Test', 'meta,link');
$this->assertEquals(
$textObj->prepValueForDB('<meta content="Keep"><link href="Also Keep">'),
$textObj->prepValueForDB('<meta content="Keep"><p>Remove</p><link href="Also Keep" />'),
'Removes any elements not in whitelist'
'<meta content="Keep"><link href="Also Keep">',
$textObj->whitelistContent('<meta content="Keep"><p>Remove</p><link href="Also Keep" />Remove Text'),
'Removes any elements not in whitelist excluding text elements'
);
$textObj = new HTMLText('Test', 'meta,link,text()');
$this->assertEquals(
'<meta content="Keep"><link href="Also Keep">Keep Text',
$textObj->whitelistContent('<meta content="Keep"><p>Remove</p><link href="Also Keep" />Keep Text'),
'Removes any elements not in whitelist including text elements'
);
}
}