Page MenuHomePhabricator

XSS using centralauthtoken and special:MyPage/apioutput.js
Closed, ResolvedPublic

Description

centralathtoken parameter allows you to log in to the api as a different user for just that request. Since each user can load custom js, this means that malicious user's custom js will be run when looking at the help page. The malicious js can make further ajax requests without the centralauthtoken parameter, thus doing actions on the original users behalf.

Attack steps:

  1. Mallory sets his [[Special:MyPage/apioutput.js]] to something like $.post( '/w/api.php?format=json&meta=userinfo|tokens&action=query', function(r) { alert(r.query.userinfo.name + ' has token ' + r.query.tokens.csrftoken ) } );
  2. Alice is logged in to wiki
  3. Alice visits Mallory's (external website)
  4. Mallory on the server side (logged into his account) does api.php?action=centralauthtoken&format=json
  5. Mallory redirects Alice to api.php?centralauthtoken=<Foo>
  6. This page loads for Alice as being under Mallory's account. Critically, User:Mallory/apioutput.js is executed
  7. The Script on User:Mallory/apioutput.js makes a post request to the API (without centralauthtoken) fetching tokens. Since Alice is looged in, this runs under Alice's account. Since this is the same origin, Mallory now has Alice's token, and can otherwise take actions on Alice's behalf

The easiest way to test this is to log in on one account to test.wikipedia.org, and a different account on say en.wikivoyage.org. Modify the js for the user being used on wikivoyage on test, Get the centralauthtoken from wikivoyage, and then load the page using the central auth token on test and watch the attack.

Event Timeline

Bawolff created this task.Sep 2 2016, 3:22 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptSep 2 2016, 3:22 AM

For starters, the whole apioutput.js is not really used. Maybe we should just kill support for it. We should also perhaps not allow centralauthtoken on the pretty-print formats.

Anomie added a subscriber: Anomie.EditedSep 2 2016, 2:10 PM

For starters, the whole apioutput.js is not really used.

apioutput.css is specifically recommended to people who want to customize the formatting of the auto-generated help, however, and the existence of XSSTC would require disabling that too.

We should also perhaps not allow centralauthtoken on the pretty-print formats.

Even if we did that, the resulting error would probably still be pretty-printed, which would defeat the purpose.

Here's a patch for CentralAuth that disables user JS (and CSS in case of XSSTC) when centralauthtoken is in use:

That should also work for 1.27. Here are untested backports for 1.26 and 1.23:

  • 1.26:
  • 1.23:
Anomie moved this task from Unsorted to Needs Review on the MediaWiki-API board.Sep 2 2016, 2:11 PM

Patch looks good to me, +2, though I'm not sure why it's not in the normal hooks file.

Anomie added a comment.Sep 7 2016, 2:46 PM

Because the hook only applies when CentralAuthTokenSessionProvider is in use, so I put it there.

It wouldn't hurt anything to put it in CentralAuthHooks beyond that if anyone looks to CentralAuthTokenSessionProvider as an example for something else they would probably miss it.

dpatrick triaged this task as High priority.Sep 13 2016, 8:55 PM

The patch looks good, but I was wondering if there was a way we could do it so that the scripts get disabled on the MW side whenever someone is overriding the session. That way we don't rely on extensions having to get it right.

I also wondering if something similar is possible with oAuth. (I have never really looked at how the custom session handling in oAuth works, so I'm not really sure).

The patch looks good, but I was wondering if there was a way we could do it so that the scripts get disabled on the MW side whenever someone is overriding the session. That way we don't rely on extensions having to get it right.

The problem there is detecting that someone is overriding the session in a dangerous way, versus in a non-dangerous way like CookieSessionProvider or BotPasswordSessionProvider. I think we'd basically need to add a SessionProvider::isDangerous() method, relying on the extension to override it when necessary.

I also wondering if something similar is possible with oAuth. (I have never really looked at how the custom session handling in oAuth works, so I'm not really sure).

Only if the attacker can force the client to send an OAuth Authorization header along with its request (see here).

This has been deployed:

22:53 dapatrick: Deployed patch for T144573 to wmf18 and wmf19

demon closed this task as Resolved.Jan 21 2017, 12:18 AM
demon claimed this task.
demon changed the visibility from "Custom Policy" to "Public (No Login Required)".

Change 333316 merged by jenkins-bot:
SECURITY: Disallow user CSS/JS when centralauthtoken is in use

https://gerrit.wikimedia.org/r/333316

Is this exploitable in 1.23? The apioutput skin didn't begin to exist until 1.25.

Change 333344 had a related patch set uploaded (by Paladox):
SECURITY: Disallow user CSS/JS when centralauthtoken is in use

https://gerrit.wikimedia.org/r/333344

Change 333345 had a related patch set uploaded (by Paladox):
SECURITY: Disallow user CSS/JS when centralauthtoken is in use

https://gerrit.wikimedia.org/r/333345

demon added a comment.Jan 23 2017, 5:12 AM

Is this exploitable in 1.23? The apioutput skin didn't begin to exist until 1.25.

I wasn't sure how far back this was exploitable, or how far back it can be fixed with the current patch.

Yeah, I was wondering because @Anomie posted a patch for 1.23.

1.23 is probably ok thanks to the lack of apioutput.js, but I'm not 100% sure.

demon added a comment.Jan 23 2017, 8:45 PM

So will the backport work on 1.27 and 1.28? If so we can merge those two

The master patch should backport easily to 1.27 and 1.28.

1.26 needed a different patch (F4426256) because it's pre-AuthManager, but it looks like we dropped support for 1.26 now?

1.23 needs another different patch (F4426258) because the file being patched got renamed at some point, unless we're going to decide not to care about it because we don't know of any attack vector pre-1.25.

demon added a comment.Jan 24 2017, 4:42 AM

The master patch should backport easily to 1.27 and 1.28.

Done.

1.26 needed a different patch (F4426256) because it's pre-AuthManager, but it looks like we dropped support for 1.26 now?

Yes, 1.26 support has ended.

1.23 needs another different patch (F4426258) because the file being patched got renamed at some point, unless we're going to decide not to care about it because we don't know of any attack vector pre-1.25.

I wouldn't worry a ton.

Change 333345 merged by jenkins-bot:
SECURITY: Disallow user CSS/JS when centralauthtoken is in use

https://gerrit.wikimedia.org/r/333345

Change 333344 merged by jenkins-bot:
SECURITY: Disallow user CSS/JS when centralauthtoken is in use

https://gerrit.wikimedia.org/r/333344