Cargo does not properly sanitize the year range filter in Special:Drilldown before it is inserted into a SELECT query, allowing for sqli in the context of the query. With the default configuration ($wgDBuser as the db user) or with an improperly scoped db user, this allows for a complete takeover of any account by exfiltrating user_token. This likely also allows for DOS by tying up db threads with expensive/slow queries.
Reproduction steps
- Create the a cargo table with a Date column and add a row to it
Template:Events
<noinclude>{{#cargo_declare:_table=Events|edate=Date}}</noinclude><includeonly>{{#cargo_store:_table=Events|edate={{{edate|}}}}}</includeonly>Event1
{{Events|edate=2020-05-01}}Note that we need at least one row in the table, so the Event1 page is needed. In practice, an anonymous user can likely create these pages, but a table with a date column and a row are reasonably likely to exist on a wiki already.
- Run the following against Special:Drilldown as a POC:
9999 - 9999) AND (1=1 (this takes about 1 second on my machine)
Special:Drilldown url: http://localhost:4000/index.php?title=Special%3ADrilldown%2FEvents&format=calendar&formatBy=edate&edate%5B0%5D=9999%20-%209999%29%20AND%20%281%3D1
9999 - 9999) OR SLEEP(0.5) AND (1=1 (this takes about 15 seconds on my machine, due to the sleep being run multiple times in a request)
Special:Drilldown url: http://localhost:4000/index.php?title=Special%3ADrilldown%2FEvents&format=calendar&formatBy=edate&edate%5B0%5D=9999%20-%209999%29%20OR%20SLEEP%280.5%29%20AND%20%281%3D1
Cause
The culprit is missing sanitization of the year range filter here. Initially I thought this affected more than just that filter because I was looking at a release branch instead of master, but on master there were recent fixes here and here for the other vectors.
It looks like there are a couple of filters that try to catch this but fail. One is this check with htmlspecialchars, which might catch a single quote breakout depending on php version, but we do not need to escape single quotes since the year is a numeric.
Then there is this set of regexs that I think tries to check for injected queries. However, this seems to fire during a second query after the injected code is executed at this line via $conds, so the injected code doesn't hit this filter before the first execution. Even with the filter, a timing attack can be executed here without using anything disallowed by the regexes.
Impact
While I couldn't output results from a SELECT subquery directly into the rendered html, access to the user table by the default db user allows a timing attack to exfiltrate data from the db. If the user_token is exfiltrated, this allows for an immediate takeover, bypassing 2fa if it exists for the account. This affects the default configuration. If Special:Drilldown is disabled then this is not exploitable as far as I could tell.
With a less privileged db user, the exfiltration might not be as big of an issue (could read private cargo data if that exists) but it's probably still a DOS vector using something like SLEEP to tie up db threads.
Patch
I think the following patch is sufficient to fix the immediate issue based on my testing, but it seems like this will always remain a serious concern if the default db user has access to the user table. My understanding is that you need higher privileges to do certain actions (create tables) but not all.