webforms filter system for descriptions doesn't work with tinymce/FCKeditor correctly

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.

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.

This patch would cause a lot of breakage in Webform, since there are a lot of situations where you need the token replacement to occur but not to be filtered. Frequent use-cases of this are when generating select-list values (which are filtered later by Drupal) or form element titles (again filtered later by Drupal).

As noted in #3, the correct thing to do is to not filter the values at all initially, then run check_markup() on the string after the tokens have been swapped in.

quicksketch,

That would make sense since pretty much anything could be typed in an input field...

Thank you for your comment!
Alexis Wilke

Closing this out, we won't be making any more 2.x or Drupal 5 versions of Webform.