Posted by kevin.dutra on September 5, 2012 at 10:48pm
1 follower
| Project: | CKEditor - WYSIWYG HTML editor |
| Version: | 6.x-1.x-dev |
| Component: | Code |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | needs review |
Issue Summary
Steps to reproduce:
- Save a node or comment using an input format that uses the "HTML filter" with some naughty HTML, like
<html><p>This text is fine.</p><script>alert('Eeek, a script!');</script></html> - Edit the node/comment you just created
- You'll notice the response gives you back the exact same text as you entered
No matter what tags you've configured as being safe for the "HTML filter", all tags will be allowed. This is due to some special logic for the "HTML filter" in ckeditor_filter_xss():
<?php
foreach ($filters as $filter) {
//built-in filter module, a special case where we would like to strip XSS and nothing more
if ($filter->module == 'filter' && $filter->delta == 0) {
preg_match_all("|</?([a-z][a-z0-9]*)(?:\b[^>]*)>|i", $text, $matches);
if ($matches[1]) {
$tags = array_unique($matches[1]);
$text = filter_xss($text, $tags);
}
}
else {
$text = module_invoke($filter->module, 'filter', 'process', $filter->delta, $filter->format, $text);
}
}
?>The logic in that
if statement doesn't make any sense to me. It basically finds all the tags in the text that was received and then tells filter_xss() that all those tags are allowed, which means that filter_xss() is going to do nothing to the string.
Comments
#1
I'm going to suggest that this logic is just plain removed and the filter is actually applied just like the other ones. Attached is a patch that does this.
If I'm missing some reason that there really should be an exception for that particular filter, please enlighten me and I'll rework the patch.
#2