Maniphest T212071

Missing CSRF token in scribunto api console
Closed, ResolvedPublic

Actions

Assigned To

Authored By

	Bawolff
	Dec 16 2018, 7:36 AM

Description

From pentest: ApiScribuntoConsole (action=scribunto-console) does not require a CSRF token.

Looking at it right now, it also accepts GET requests.

I'm not sure this is an actual issue:

Scribunto sessions are extremely ephemeral.
What would an attacker even gain by messing with someone's scribunto debug session?
Each session has a 31 bit (non-cryptographically secure. i.e. generated via mt_rand()) random identifier associated with it. The attacker would have to be able to guess this. Given its only 31 bits, and its not securely generated, this isn't outside the realm of possibility, but it also makes things harder to pull off.

That said, no harm in doing things properly. So I think we should do the normal, require CSRF token and make this module must POST.

Details

	Subject	Repo	Branch	Lines +/-
	Require CSRF token for action=scribunto-console	mediawiki/extensions/Scribunto	master	+5 -1

Customize query in gerrit

Event Timeline

Bawolff created this task.Dec 16 2018, 7:36 AM

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptDec 16 2018, 7:36 AM

Bawolff added a project: Scribunto.Dec 16 2018, 7:36 AM

I'm not sure this is an actual issue:

I don't think it is.

Scribunto sessions are extremely ephemeral.

Indeed. It's not saved anywhere persistent client-side so it's gone as soon as the person leaves or reloads the page, plus it's thrown away if the user clicks the "Clear" button or changes the textarea in the edit form.

What would an attacker even gain by messing with someone's scribunto debug session?

The "session" here is just the history of commands previously typed into the console and a copy of the previous value of the content parameter, so they don't have to be repeated for every request. The best an attacker could do would be to screw up the Lua code being run, either to confuse someone or to hope they're going to copy-paste output.

Each session has a 31 bit (non-cryptographically secure. i.e. generated via mt_rand()) random identifier associated with it. The attacker would have to be able to guess this. Given its only 31 bits, and its not securely generated, this isn't outside the realm of possibility, but it also makes things harder to pull off.

Note the cache key also includes the user's ID. So the attacker would either have to also be able to spoof the user somehow or restrict the attack to people using the console while logged out.

We generally require POSTs and CSRF tokens for actions that actually write the wiki. This doesn't, other than that ephemeral state cache.

• chasemp added a project: Security.Feb 10 2020, 11:00 PM

• chasemp removed a project: acl*security.Feb 20 2020, 8:08 PM

Aklapper removed a subscriber: Anomie.Oct 16 2020, 5:02 PM

@sbassett, @Reedy: two questions, 1) this seems very unexploitable, can we make it public? 2) Do you all have an opinion on whether this should require a CSRF token? It's an internal API module so we can make the change without having to bother with the breaking change process.

In T212071#8254923, @Legoktm wrote:

this seems very unexploitable, can we make it public?

Given @Bawolff and Anomie's reasoning above, I think that's fine.

Do you all have an opinion on whether this should require a CSRF token? It's an internal API module so we can make the change without having to bother with the breaking change process.

IMO if we're going to expend energy to fix an issue like this, we should just go ahead and require the token unless there is fear that it will break several existing workflows, etc. Digging around in turnilo, it seems action=scribunto-console endpoints get a pretty small amount of traffic each month, like under 2k hits, most likely.

sbassett lowered the priority of this task from Low to Lowest.Sep 27 2022, 5:07 PM

sbassett changed the visibility from "Custom Policy" to "Public (No Login Required)".

Change 836962 had a related patch set uploaded (by Legoktm; author: Legoktm):

[mediawiki/extensions/Scribunto@master] Require CSRF token for action=scribunto-console

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

gerritbot added a project: Patch-For-Review.Sep 30 2022, 12:59 AM

Scripts to fix: https://global-search.toolforge.org/?q=%22scribunto-console%22&namespaces=2%2C4%2C8&title=%28Gadgets-definition%7C.*%5C.js%29

https://meta.wikimedia.org/w/index.php?title=Tech%2FNews%2F2022%2F40&type=revision&diff=23872498&oldid=23871998

I'm pretty concerned that there are gadgets that are calling this API :/ will file a separate task for obsoleting libLua.js.

Anyways, I double-checked, passing the token when it's not needed just results in an API warning:

"warnings": {
  "main": {
    "*": "Unrecognized parameter: token."
  }
},

so it's safe for people to update to using .postWithToken( 'csrf', params ) ASAP.

Legoktm claimed this task.Sep 30 2022, 1:29 AM

Quiddity moved this task from To Triage to In current Tech/News draft on the User-notice board.Sep 30 2022, 7:59 PM

Change 836962 merged by jenkins-bot:

[mediawiki/extensions/Scribunto@master] Require CSRF token for action=scribunto-console

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

ReleaseTaggerBot added a project: MW-1.40-notes (1.40.0-wmf.5; 2022-10-10).Oct 5 2022, 3:00 PM

Maintenance_bot removed a project: Patch-For-Review.Oct 5 2022, 3:30 PM

Quiddity moved this task from In current Tech/News draft to Already announced/Archive on the User-notice board.Oct 6 2022, 11:37 PM

Not planning to do any backports given the low risk balanced with changing an API.

Maintenance_bot edited projects, added User-notice-archive; removed User-notice.Oct 17 2022, 4:30 PM

In T212071#8274383, @Legoktm wrote:

Scripts to fix: https://global-search.toolforge.org/?q=%22scribunto-console%22&namespaces=2%2C4%2C8&title=%28Gadgets-definition%7C.*%5C.js%29

I went through and fixed all of these.

In T212071#8355028, @MrStradivarius wrote:

In T212071#8274383, @Legoktm wrote:

Scripts to fix: https://global-search.toolforge.org/?q=%22scribunto-console%22&namespaces=2%2C4%2C8&title=%28Gadgets-definition%7C.*%5C.js%29

I went through and fixed all of these.

Thanks!

Missing CSRF token in scribunto api console Closed, ResolvedPublic Actions

Description

Details

Event Timeline

Missing CSRF token in scribunto api console
Closed, ResolvedPublic

Actions