e107inc/e107: e107 Search Author Reflected XSS Report

Summary

Anyone, without an account, can put markup and script of their choosing into the site's search page. They send a link, or a page that submits a form to the site, and the script runs in the browser of whoever opens it. It runs in the site's own origin, with that person's session.

The search page draws an Author field for search areas that offer one. It passed the author value from the address to the form helper as the field's options, not as its value. The helper reads a string of options the way a query string is read, and takes a bracketed parameter as a ready-made list, so the sender chooses every option of the field, names as well as values. Several of those options reach the page without escaping.

When the person who opens the link is a logged-in administrator, the script acts as that administrator. An administrator can install a plugin, so this amounts to running code on the server.

Is my site affected, and what can I do now?

The check. On the site's search page, switch to the advanced options for each search area. If any area offers an Author field, the site is exposed while it runs an affected version. The comments search offers one, and so do the Download, Forum, Chatbox and FAQ plugins' search areas.

On releases up to 2.3.12 the advanced options are only served for a search area that the search configuration lists. Opening Admin > Search once is enough to list every area, so a site where an administrator has ever opened that page should be treated as exposed.

Until the site is updated to 2.3.13, either of these closes it:

1. Apply the fix by hand. In search.php, the Author field is drawn by a call

ending userpicker(...,$adv_key,$_GET[$adv_key]);. Remove the last argument, leaving userpicker(...,$adv_key);. The field renders exactly as before for every genuine request.

1. **Strip every query parameter whose name starts with author** from requests to

search.php, at the web server or a proxy in front of it. Match the name after URL-decoding, and include bracketed names such as author[...]: those arrive as a list of options and need no = in the value. A rule that only looks at a plain author= parameter leaves the site exposed. The search form never relies on these parameters (its field, author_name, is ignored by every search area), so nothing a visitor does from the page is lost.

Details

The advanced block draws an Author field with the request value as the third argument. On 2.3.12:

https://github.com/e107inc/e107/blob/ffb1c81938deaaa9bf1f897a409b181e0294e450/search.php#L474

The development branch has the same call, reworked but with the same arguments:

https://github.com/e107inc/e107/blob/fa4bd2a97da1d0793388384272063516bfd5df1d/search.php#L483

e_form::userpicker() is declared ($name, $value, $options). A string in $options goes through parse_str(), which decodes the value once more and turns it into option keys as well as values; a bracketed author[...] parameter arrives as a list and is used as it is:

https://github.com/e107inc/e107/blob/ffb1c81938deaaa9bf1f897a409b181e0294e450/e107_handlers/form_handler.php#L2009-L2014

The options are merged over the picker's own and handed to e_form::text():

https://github.com/e107inc/e107/blob/ffb1c81938deaaa9bf1f897a409b181e0294e450/e107_handlers/form_handler.php#L2094-L2096

From there, four of them reach the page unsafely.

• **tabindex** is in the default option set and is placed inside single quotes as

it stands, so a quote in the value ends the attribute:

https://github.com/e107inc/e107/blob/ffb1c81938deaaa9bf1f897a409b181e0294e450/e107_handlers/form_handler.php#L4609 https://github.com/e107inc/e107/blob/ffb1c81938deaaa9bf1f897a409b181e0294e450/e107_handlers/form_handler.php#L4430-L4434

• **other** is in the default option set too and is appended verbatim, which is

enough to add attributes, close the tag and write elements after it:

https://github.com/e107inc/e107/blob/ffb1c81938deaaa9bf1f897a409b181e0294e450/e107_handlers/form_handler.php#L4613 https://github.com/e107inc/e107/blob/ffb1c81938deaaa9bf1f897a409b181e0294e450/e107_handlers/form_handler.php#L4446-L4450

• **Any key beginning data-** is kept, and the attribute writer escapes its value

but writes its name as given, so the name can carry further attributes:

https://github.com/e107inc/e107/blob/ffb1c81938deaaa9bf1f897a409b181e0294e450/e107_handlers/form_handler.php#L4571 https://github.com/e107inc/e107/blob/ffb1c81938deaaa9bf1f897a409b181e0294e450/e107_handlers/e_parse_class.php#L2157

• **selectize** replaces the picker's whole configuration, which text() hands to

the page's scripts. The bundled picker library treats some of those settings as markup, so script runs as the page loads without the visitor touching the field:

https://github.com/e107inc/e107/blob/ffb1c81938deaaa9bf1f897a409b181e0294e450/e107_handlers/form_handler.php#L874-L882

The tag itself is assembled here:

https://github.com/e107inc/e107/blob/ffb1c81938deaaa9bf1f897a409b181e0294e450/e107_handlers/form_handler.php#L906

So escaping individual options would not have been enough. The fault is that the address chose the options at all.

Two ways in, both unauthenticated. The first is an ordinary GET of the search page. The second needs no token, no cookie and no X-Requested-With header: the search page's constructor echoes the advanced block on its own for any request the site treats as AJAX, and a request declares itself AJAX with an ordinary parameter. A cross-site form POST is a top-level navigation, so the fragment is parsed as a page in the site's origin.

https://github.com/e107inc/e107/blob/ffb1c81938deaaa9bf1f897a409b181e0294e450/search.php#L86-L94 https://github.com/e107inc/e107/blob/ffb1c81938deaaa9bf1f897a409b181e0294e450/e107_handlers/e107_class.php#L5123

e107's built-in request filtering did not stop it in testing, and the site sends no Content-Security-Policy header.

Why 2.2.0. The call was written in 2014 for an older userpicker() whose third parameter was a default name. The method took its present signature in 2016, but until 2019 it read only three options of its own and dropped the rest. From the change that passed the remaining options on to the input (a6476e680e, first released in 2.2.0), the address has decided what that input carries. The same mismatch is why searching by author has never worked in those releases: the form sends author_name while every search area reads author.

Reproduced on 2.3.12, configured as above, and on both development branches as freshly installed: the injected markup was in the served page, and the script ran in headless Chrome. On the development branch, the script also ran in a logged-in administrator's browser and read an administration page and its form token, and the picker-settings route ran script as the page loaded.

Impact

Cross-site scripting, reflected, against anyone who follows a link or lands on a page that submits a form for them. No account is needed to send it.

The script runs in the site's origin with the victim's session. Against an administrator it can read and submit any administration form, including the ones that install plugins, which means running code on the server. Against a member or a guest it can act in their name and change what the page shows them.

Severity

CVSS:3.1/AV:N/AC:L/PR:N/UI:R/S:C/C:H/I:H/A:H, 9.6 Critical.

The impact is scored as code execution because the script can act as an administrator, and an administrator can install code. UI:R because the victim has to open the link. AC:L despite the precondition on released versions: a search page with a configured area is the ordinary state of a running site.

Patches

The search page now passes the author field its name and value only, so the address can no longer choose its options. e_form::get_attributes() also writes tabindex through the shared, escaping attribute writer as an integer. An integer tabindex renders exactly as before.

other, data- attribute names and the picker's selectize settings are left as they are on purpose. other is documented as a raw attribute string, and core's own administration lists pass inline styles through it. data- names and picker settings are chosen by the code that calls the helper, and core's callers pass fixed ones. Changing how any of them is written would change what existing callers render. The fault was a caller handing them request data, and that is what changed.

Regression tests render the search page with hostile author values and check both the advanced block and the settings handed to its scripts, and check that a tabindex cannot leave its attribute.

Fix commits: release/v2.3.x ee474c7415; master e9416ba0c0.