I audited the api modules in core, and a few others will give back login or anonymous edit tokens too. These need to be fixed before we can effectively fix bug 38417.

• ApiTokens
  • curl -i 'http://localhost/wiki/api.php?action=tokens&type=edit&format=json&callback=foo'

• ApiUnblock
  • curl -i --data 'action=unblock&user=User1&gettoken=true&format=json&callback=bar' 'http://localhost/wiki/api.php'

• ApiLogin.php
  • curl -i --data 'action=login&format=json&lgname=foo&callback=bar' 'http://localhost/wiki/api.php'

• ApiCreateAccount.php
  • curl -i --data 'action=createaccount&format=json&name=foo&callback=bar' 'http://localhost/wiki/api.php'

• ApiBlock.php
  • curl -i --data 'action=block&user=User1&gettoken=true&format=json&callback=bar' 'http://localhost/wiki/api.php'

I'm planning to look at this tomorrow morning. Current thoughts are that the block and unblock 'gettoken' parameters have been deprecated since 1.20, so they are ripe for removal. The others will get a check similar to that currently used for prop=info.

I also grepped through the WMF-deployed extensions. They all seem to use the generic edit token from action=tokens or prop=info, or use the hook to add to action=tokens, so that part of it looks ok.

I also note that ApiQueryDeletedrevs could give a token, but since anons don't typically have 'deletedhistory' it's not likely to actually be gettable via jsonp.

(In reply to comment #2)

I'm planning to look at this tomorrow morning. Current thoughts are that the
block and unblock 'gettoken' parameters have been deprecated since 1.20, so
they are ripe for removal. The others will get a check similar to that
currently used for prop=info.

If we want to fix 38417 for REL1_19/REL1_20, then we'll probably need a patch that either checks for the callback param, or moves the gettoken handling after the permission checks. Either way is a pretty simple patch. But yes, we should remove the code altogether for 1.22 I think.

Created attachment 12450
Patch against master

attachment patch-1.22.diff ignored as obsolete

Created attachment 12451
Patch against REL1_21

attachment patch-1.21.diff ignored as obsolete

Created attachment 12452
Patch against REL1_20

attachment patch-1.20.diff ignored as obsolete

Created attachment 12453
Patch against REL1_19

attachment patch-1.19.diff ignored as obsolete

Created attachment 12456
Patch against master

Attached:

Created attachment 12457
Patch against REL1_21

Attached:

Created attachment 12458
Patch against REL1_20

Attached:

Created attachment 12459
Patch against REL1_19

Attached:

Thanks Brad! I've tested the patch against master, and it seems to be working well and prevents the leak. As mentioned on irc, the action=tokens api returns the error "Unrecognized value for parameter 'type'" when the callback is added, instead of an explicit message about the callback parameter. But since this is the same way ApiQueryInfo currently works, it seems like api users should be able to figure it out.

Roan, can you take a look at the patches, and verify they look sane to you, or if there is anything we're missing by doing it this way?

Roan approved on irc. Deployed into production on Aug 29.

Change 82527 merged by jenkins-bot:
SECURITY: Prevent tokens in jsonp mode

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

Change 82537 had a related patch set uploaded by CSteipp:
SECURITY: Prevent tokens in jsonp mode

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

Change 82541 had a related patch set uploaded by CSteipp:
SECURITY: Prevent tokens in jsonp mode

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

Change 82545 had a related patch set uploaded by CSteipp:
SECURITY: Prevent tokens in jsonp mode

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

Change 82537 merged by CSteipp:
SECURITY: Prevent tokens in jsonp mode

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

Change 82545 merged by jenkins-bot:
SECURITY: Prevent tokens in jsonp mode

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

Change 82541 merged by jenkins-bot:
SECURITY: Prevent tokens in jsonp mode

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

[restoring RESOLVED FIXED state which was set before the Gerrit Notification Bot inserted links to the Gerrit patchsets]

This issue was assigned CVE-2013-4302