Page MenuHomePhabricator

ISA tool is missing CSRF protection
Closed, ResolvedPublicSecurity

Description

The /api/post-contribution route of the ISA Toolforge tool combines several antipatterns:

  1. It doesn’t have any CSRF protection, as far as I can see: there’s no kind of edit token in the request data, nor does it check where the request comes from.
  2. The parameters of the API call made to the MediaWiki Action API are part of the request data (generated by the ISA JavaScript code in the benign case, but otherwise attacker-controlled), the Python backend mainly adds a valid CSRF token.
  3. It’s registered not only for the POST method, but also for GET. (But it also only reads only the request body, not the URL, so I don’t think it can actually be exploited over GET.)

Issue 1 means that if a user is logged into ISA, any website they visit can post contributions on their behalf. Issue 2 means that the contributions are not limited to the kinds of edits that the tool is supposed to make, but that any API action is available, as long as it a) uses the default csrf token type (unlike e.g. the rollback API), and b) is covered by the grants of the tool’s OAuth consumer (Interact with pages: Edit existing pages; Create, edit and move pages).

Example:

fetch('https://isa.toolforge.org/api/post-contribution', {
  method: 'POST',
  mode: 'cors',
  credentials: 'include',
  body: JSON.stringify([{
    campaign_id: 0,
    image: null,
    edit_action: null,
    edit_type: null,
    country: null,
    api_options: {
      action: 'edit',
      title: 'User:Lucas Werkmeister/sandbox',
      appendtext: 'Hi :)',
    },
  }]),
});

I ran this code on tmp.lucaswerkmeister.de (an arbitrary website which conveniently has no connect-src CSP that might block this request), and an edit was made.

Issues 1 and 3 also seem to affect several other APIs of the tool (e.g. creating or updating a campaign), but I haven’t bothered checking if those APIs are actually vulnerable.

Event Timeline

@Huji @Ladsgroup @Matanya @Quiddity @zhuyifei1999 Hello, I'm pinging you all as members of the Toolforge standards committee, following the instructions for handling security issues in Toolforge tools. It looks none of you were added here yet, so not sure if you're aware.

I'm also pinging the maintainers (@Eugene233 @NavinoEvans) one more time -- could you please have a look at this task soon? It's possible the OAuth consumer will get disabled if the CSRF vulnerability is not fixed in reasonable time.

This comment was removed by Anthere.

Sebastian Berlin <sebastian.berlin@wikimedia.se> will start to work on the tool very soon. He said he would start to look at this issue first thing.

He would like to be granted access to this task. Can someone involved here give him access ? His phabricator account is https://phabricator.wikimedia.org/p/Sebastian_Berlin-WMSE/

Thanks

Sebastian_Berlin-WMSE changed the task status from Open to In Progress.Nov 4 2021, 10:18 AM

I added CSRF-tokens using Flask WTF. This should cover the whole API. I tested and example now gives and error, but editing using the tool works. I'll have a look at issue 2 next.

@Huji @Ladsgroup @Matanya @Quiddity @zhuyifei1999 Hello, I'm pinging you all as members of the Toolforge standards committee, following the instructions for handling security issues in Toolforge tools. It looks none of you were added here yet, so not sure if you're aware.

I'm also pinging the maintainers (@Eugene233 @NavinoEvans) one more time -- could you please have a look at this task soon? It's possible the OAuth consumer will get disabled if the CSRF vulnerability is not fixed in reasonable time.

I've uploaded patches. @NavinoEvans has started reviewing. Anyone else is welcome to also have a look.

The tool has been updated with the three patches for this task. I don't know if that's enough to resolve it or if there are any remaining steps.

I can confirm the steps to reproduce in the description no longer work. The response is 400 Bad Request, with an informational error message in the response body. So, fixed for the CSRF tokens side.

However...why is the application writable? The whole directory at /data/project/isa/www/python/src/isa is 777, which means any Toolforge user can change any file in that directory (I successfuly reproduced that with messages.pot, where I added a comment saying "I was here --Urbanecm"). This was possible even though messages.pot had 644 (owner can read+write, everyone else can just read), because in Unix systems, to delete a file, you need write permissions to the directory, not to the file.

Thanks for pointing this out. I've changed the permissions for all files to o-w.

Was there anything else that was needed before this task can be resolved?

Probably not, the fixes look alright to me.

Can someone with the appropriate rights make the task public now?

Urbanecm changed the visibility from "Custom Policy" to "Public (No Login Required)".Mar 5 2022, 10:09 AM
Urbanecm changed the edit policy from "Custom Policy" to "All Users".

Can someone with the appropriate rights make the task public now?

Done