webforms filter system for descriptions doesn't work with tinymce/FCKeditor correctly
marcingy - April 6, 2008 - 17:57
| Project: | Webform |
| Version: | 5.x-2.0-beta3 |
| Component: | Code |
| Category: | feature request |
| Priority: | normal |
| Assigned: | marcingy |
| Status: | needs work |
Description
If tinymce is used to format the description text within a form formatting is not preserved by the webform filter call. This is because the call to filter_xss always uses the default filter list. This removes the
tags placed by tinymce. Instead a call should be made to check_markup to allow for variable html filter lists.
This change may justify the revision of my previous filter patch as a call to check_markup is no longer made in the description_filter function. If tinymce is not used the current filter process works correctly
| Attachment | Size |
|---|---|
| webform.filter.patch | 937 bytes |
#1
change to correct status
#2
New version to fix issue with emails etc.
#3
It doesn't look like this was ever addressed, sorry for the lack of response. I'd prefer not to make a special argument just for "description", though this second patch takes into consideration the appropriate precautions of filtering. The _webform_filter_values() function is used all over the place to remove HTML code from select list values, descriptions, email subjects, names, and several other places. Using filter_xss is both safer and more appropriate in most of these cases.
So for descriptions, what I think would be best would be to call _webform_filter_descriptions() with $strict == FALSE and then running check_markup() on the result.
#4
I just posted some info in another entry (see #434914: Security questions: Markup component and others for non-admins) and here I post the patch.
My point of view is that if you read data from variables such as $_GET and $_POST you ought to filter them, just in case. Those are potentially from external users and not filtering them is dangerous.
My fix is to always apply (no $strict flag) the filter_xss() over the variable values.
I may be missing something since the function may expect to have the entire $string parsed via filter_xss() all the time. In that case, you may want to add a test on $strict in the variable and apply the filter only if it is not going to be applied at the end of the function. (i.e.
$replace[] = $strict ? $value : filter_xss($value);and leave the return statement as it is.)IMPORTANT NOTE: if you keep the $strict flag, you need a fix like marcingy's that makes sure the filter_xss() is not applied to the entire string since the description code will anyway apply the check_markup() code.
This is not a problem with the description but a prefill of a field with $_GET or $_POST. A hacker could tell you to go to a website with a "random" webform, and there he has a $_GET that reacts "badly" in one of those fields...
Thank you.
Alexis Wilke
P.S. This patch is for 6.x-2.7, not 5.x which is why I did not change anything in the issue settings.