Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.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():
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.
| Comment | File | Size | Author |
|---|---|---|---|
| #1 | filtered-html-1775840-1.patch | 960 bytes | kevin.dutra |
Comments
Comment #1
kevin.dutra CreditAttribution: kevin.dutra commentedI'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.
Comment #2
kevin.dutra CreditAttribution: kevin.dutra commentedComment #2.0
kevin.dutra CreditAttribution: kevin.dutra commentedCorrecting name of the filter.
Comment #3
kevin.dutra CreditAttribution: kevin.dutra commentedResolved by CVE. https://www.drupal.org/node/2357029
Comment #4
jcisio CreditAttribution: jcisio commentedThanks kevin for the update.
I indeed saw this issue two years ago, but didn't have time to test. But I don't think it is the same as the CVE. The module does intentionally not filter the SCRIPT tag because CKEditor can handle it well (it "blocks" the tag, but preserves it). The CVE is about an attack through the result from the XSS filter itself (just text, without CKEditor). However, in this CVE, this issue is fixed (even not tested) because as a mesure of security, SCRIPT is filtered out now (content is lost, for better security).