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

AttachmentSize
webform.filter.patch937 bytes

#1

marcingy - April 6, 2008 - 18:20
Status:active» needs review

change to correct status

#2

marcingy - April 6, 2008 - 19:01

New version to fix issue with emails etc.

AttachmentSize
webform.filter2.patch 1.41 KB

#3

quicksketch - October 30, 2008 - 23:01
Status:needs review» needs work

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

AlexisWilke - June 22, 2009 - 00:41
Title:webforms filter system for descriptions doesn't work with tinymce correctly» webforms filter system for descriptions doesn't work with tinymce/FCKeditor correctly

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.

AttachmentSize
webform-6.x-filter_description.patch 608 bytes
 
 

Drupal is a registered trademark of Dries Buytaert.