Problem/Motivation
In FilterPluginBase::convertExposedInput(), when the filter is a grouped exposed filter (isAGroup()), the selected group value is read from user input:
elseif (isset($input[$this->options['group_info']['identifier']])) {
$selected_group = $input[$this->options['group_info']['identifier']];
}
Nothing guarantees that $selected_group is a scalar. If a user (or a malicious client) submits this parameter as an array — e.g. ?group=[] or ?group[]=foo — $selected_group becomes an
array. That array is then used as an array offset further down:
if ($selected_group != 'All' && empty($this->options['group_info']['group_items'][$selected_group])) {
which throws:
TypeError: Cannot access offset of type array in isset or empty() in Drupal\views\Plugin\views\filter\FilterPluginBase->convertExposedInput()
This code path is invoked from ViewExecutable::_build() on $this->exposed_data (which can come directly from the request's query parameters), so it can be hit before any form validation
(validateExposed()) runs, on any public view with a grouped exposed filter.
Steps to reproduce
1. Create a view with an exposed filter, with the "Grouped filters" (grouped exposed filter) option enabled.
2. Load the view's page passing the group identifier as an array, e.g. ?[]=1 or ?[]=1&[]=2.
3. Observe the fatal TypeError (white screen / 500 error depending on the environment).
Proposed resolution
Check that $selected_group is actually a scalar before using it as an array offset, and return FALSE (no valid selection) otherwise — the same defensive behavior already applied for other
malformed-input cases in this method.
Remaining tasks
User interface changes
Introduced terminology
API changes
Data model changes
Release notes snippet
Issue fork drupal-3617858
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #4
jeffmattsonThe MR is pointed at Main. I think it should be pointed at 11.3.x.
Comment #5
nicolas s. commentedthanks jeffreysmattson, I change the destination of MR to pointed at 11.3.x
Comment #6
nicolas s. commentedComment #7
smustgrave commentedThank you for reporting. Fixes need to land in main first and could be backported. Though 11.3.x I believe is just security coverage now.
Also as a bug will need a test case please
Thanks.
Comment #8
jeffmattsonI am confused. When working on an issue, what branch are you supposed to merge it into. Say if the issue is in 11? You merge it into main and then backport to 11? Is main the "develop" branch here? I have never worked in an environment where fixes merge right into main. Not saying it's wrong. Just different.
Comment #9
nicolas s. commentedI've updated this merge request to target main instead of 11.3.x.
I also fixed the source branch, which wasn't originally branched from main.
Rebasing it kept generating a large number of conflicts, and the MR was showing 500+ commits instead of just the intended one.
The branch has now been recreated properly from main, so the diff should be clean and limited to the actual change.