Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.In the latest version of D7, try creating a text format with the "escape all HTML" filter applied.
Then try creating a node with any HTML in it that you want. The HTML passes through, completely unescaped.
Not sure if it's related or not, but looking in the database, I noticed that in the {filter} table the "module" column gets left blank for the filter I created during the above.
| Comment | File | Size | Author |
|---|---|---|---|
| #6 | filter-escape-html-2.patch | 3.22 KB | David_Rothstein |
| #2 | drupal.filter-escape-html.patch | 620 bytes | sun |
Comments
Comment #1
David_Rothstein commentedTo clarify, by the way, this bug only occurs in the unstable Drupal 7.x-dev version, not Drupal 5 or Drupal 6 -- which is the only reason it's OK to report it publicly (since this is clearly a security issue).
Comment #2
sunIt seems this filter got lost in #546336: hook_filter_info(): Remove $op from hook_filter(), because it did not have an own callback function.
Comment #3
heine commentedWow, great catch! And it begs the question, why was there no test? It certainly needs one.
Comment #4
webchickWe most absolutely, definitely need a test for this. Egads.
Comment #5
sunTagging.
Comment #6
David_Rothstein commentedIt turns out there was almost a test, but it explicitly tested check_plain() directly rather than the filtering function itself... Evidently we need to test both.
Also note that the latest patch at #11218: Allow default text formats per role, and integrate text format permissions contains a test that would catch this too, although at a higher level.
Comment #7
David_Rothstein commentedI was wondering how a missing function could go completely unnoticed, and it turns out it's because of code like this in check_markup():
The function_exists() is presumably there because you can't guarantee that the module which provides the filter is enabled... but this whole thing is a little sketchy. When you disable a module that provides a filter, it can really screw up your text formats - I'm not sure that should fail silently.
Perhaps we need a followup issue to explicitly disable filters in the database when the module that provides them is disabled (and then the site admin could be warned of the consequences of that right at the same time). Then we could remove the function_exists() check from the above code and help prevent bugs like this from slipping in again?
Comment #8
sunLooks good.
Created #562694: Text formats should throw an error and refuse to process if a plugin goes missing to tackle the clusterfuck.
Comment #9
sunReady to fly.
Comment #10
webchickWhew! Thanks a lot, folks. Committed to HEAD.
What's up with the unpublished comments..?
Comment #11
sun@webchick: See my post to the security list. Actually, it's possible that this issue has an high impact on #11218: Allow default text formats per role, and integrate text format permissions.
Comment #12
David_Rothstein commentedRight, @sun unpublished comments #7 and #8 in order to check with the security team if #562694: Text formats should throw an error and refuse to process if a plugin goes missing needed to be handled privately, but the conclusion after some discussion was that it's OK to work on this as a security hardening patch in the public issue queue, so I've republished those comments now.
I'm very interested to hear any opinions about the impact of this on #11218: Allow default text formats per role, and integrate text format permissions, obviously :)