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.In #406580: XSS vulnerability in userreference / nodereference code has been added to the optionwidget select lists to remove HTML and leave entities unencoded.
This is done with the usage of array_map:
$options = array_map(create_function('$opt', 'return html_entity_decode(strip_tags($opt), ENT_QUOTES);'), $options);
This code perfectly works for CCK and the optionwidgets, but the array_map has one big disadvantage: it doesn't work with
nested arrays. I know, that the optionwidgets module doesn't use nested arrays, but one of my modules Content Taxonomy, which uses
the optionwidgets module to display select/checkboxes/radio form elements, has a feature to display optgroups in select
lists. For this it uses a nested array. Since the changes, the optgroups are destroyed by array_map.
To fix this, it would be possible to either replace the array_map with array_walk_recursive, but array_walk_recursive is PHP 5 only, so no
option here. The other possibility is to add a recursive function to the optionwidget module, which walks through the array
and sanitizes the code (see my patch).
There is also an feature request #306930: Support optgroups in allowed values to support opt groups, which would require this fix.
Matthias
| Comment | File | Size | Author |
|---|---|---|---|
| #9 | content_allowed_values_flatten-521002-9.patch | 6.54 KB | markus_petrux |
| #5 | content_allowed_values_filter_html-521002-5.patch | 2.98 KB | markus_petrux |
| optionwidgets_patch_array_map.patch | 1.14 KB | mh86 |
Comments
Comment #1
markus_petrux CreditAttribution: markus_petrux commentedI would like to wait for yched input here, but this fix makes sense to me.
The fix ought to be applied to content_handler_filter_many_to_one as well, though.
One doubt I have is whether the optionwidgets_filter_html() function in the patch should be part of optionwidgets module, or part of content module. If the later, then it could be placed next to content_allowed_values(), which is related, and maybe the funcion name could something like content_allowed_values_filter_html()?
Comment #2
yched CreditAttribution: yched commentedThe change looks fine. We should add a comment the point is to support selects with 'optgroups'.
About where that function should live: well, it's fairly tied to the fact that we're dealing with a 'select' input, which does not allow HTML or encoded entities. So my first thought would be to leave it in optionwidgets.
After this is fixed in D6, we'll need a D7 'Fields in core' issue to forward port the change.
Comment #3
markus_petrux CreditAttribution: markus_petrux commentedThe problem I see in keeping optionwidgets_filter_html() as part of optionwidgets module is that maybe this module is not enabled, while the many_to_one views handler is used by another field module, and since we also need to do the job there, we will have to load optionwidgets module if it is not enabled.
We have an example in content_taxonomy. It reuses the many_to_one filter handler provided by CCK. You may have installed content taxonomy module with the tree widget only. Here optionwidgets is not required, so the filter will WSOD if we do not move optionwidgets_filter_html() to content module, and then this funcion needs another name.
We can document in the doxygen of that function the fact it is used in views many to one filter handler and that this handler may be used by other modules while optionwidgets is not required.
Comment #4
markus_petrux CreditAttribution: markus_petrux commentedWell, if no one minds, I think I'll proceed as in #1. Basic reason is we may need this funcion when optionwidgets is not installed. More in comments #1 to #3.
Comment #5
markus_petrux CreditAttribution: markus_petrux commentedSo, how about this?
Comment #6
markus_petrux CreditAttribution: markus_petrux commentedCommitted the patch in #5 to CCK2 and CCK3.
Comment #7
markus_petrux CreditAttribution: markus_petrux commentedBTW, better title...
Comment #8
markus_petrux CreditAttribution: markus_petrux commentedOops! Adding support for optgroups was not as easy as it seemed. Please, see: #546148: Validation error on numeric select's using optgroup.
That is, when content_allowed_values() is invoked to validate, it needs to return a flatten array. We could add a second argument ($flatten = TRUE) to content_allowed_values, and set it to FALSE when retrieving values for the options in the select widget. When $flatten is TRUE, we could parse the array recursively to return an array with a single dimension.
@yched: sounds good?
Comment #9
markus_petrux CreditAttribution: markus_petrux commentedWell, here's a patch that tries to fix this issue when multidimensional arrays are used to add optgroups to select widgets.
The patch adds a new function to content.module: content_array_flatten(). This function is used by content_allowed_values() that gets a new optional argument $flatten, which is TRUE by default to make sure we always get a single dimension (flatten) array, which is almost always.
Then, the many to one filter in Views when used against a select widget, and the same for the optionwidgets module, when they invoke content_allowed_values(), they use the second argument to say No, I'm fine with a multidimensional array.
This needs testing with all kinds of optionwidgets, and views using filters for them.
Comment #10
markus_petrux CreditAttribution: markus_petrux commentedCommitted to CCK2 and CCK3.