Page MenuHomePhabricator

URL Shortener Should Not Mystery Meat Important Links
Open, HighPublicBug

Description

Consider this link (DO NOT CLICK): http://w.wiki/zL -> it points to Special:UserLogout.

Shortened URLs should NOT take action on user's behalf without them knowing what it does.

Steps to Reproduce:

Click on a shortened link that goes to a user action.

Actual Results:

The action happens.

Expected Results:

Mediawiki should protect me from actually having those things happen without me having to do it.

Event Timeline

Jorm created this task.Apr 14 2019, 8:11 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptApr 14 2019, 8:11 PM
Jorm added a comment.Apr 14 2019, 8:14 PM

There's probably a LOT of action urls that should be blacklisted. A trap would be someone creates a link that executes an action that has higher privileges and tricks a sysop into clicking it, etc.

There's probably a LOT of action urls that should be blacklisted. A trap would be someone creates a link that executes an action that has higher privileges and tricks a sysop into clicking it, etc.

Besides logout (which has been known about for years and needs to be fixed >.<), no other action URL should be taking action upon a GET request, they should all require POST requests with tokens, otherwise it's basically an XSS.

Jorm added a comment.Apr 14 2019, 8:34 PM

"Should" and "Does" are two different things, though. The fact that something as basic as "Logout" has this flaw does not inspire me with confidence that other, more important things are not broken.

I'd advise against making shortened URLs for anything in the Special: name space. It's a simple, easy-to-understand fix, and I'm not sure of a use case where Special pages absolutely NEED to be shortened.

You can also add a link that log out the user in any wiki page by inserting [[Special:UserLogout|click me]] code. It is also possible to add it as external link like [https://en.wikipedia.org/w/index.php?title=Special:UserLogout click me too].

Note that O've discovered that for some reason Special:UserLogout as redirect target is blacklisted, see this test. So we can say that in most of the cases user can see in its browser interface the link target and for example see before clicking on it that it'll log the user out. UrlShortener ext allows to obfuscate that kind of links.

As said by Legoktm mediawiki will not allow users to take actions (edit, block...) without clicking on "Submit" of a form. See https://en.wikipedia.org/wiki/Cross-site_request_forgery.

You can also have the need to short some special urls like for example https://foundation.wikimedia.org/w/index.php?title=Special:Log&offset=20170710145330&type=newusers&user=&page=&wpdate=&tagfilter=&subtype= (yes it is possible to remove unnecessary parameters but not logical and user-friendly).

I'd be happy with exceptions like Special:Watchlist (w.wiki/zf), Special:Random (w.wiki/$F), Special:RecentChanges (w.wiki/$G), Special:MyPage (w.wiki/$H) etc. The problematic special pages are ones that invoke user actions.

"Should" and "Does" are two different things, though. The fact that something as basic as "Logout" has this flaw does not inspire me with confidence that other, more important things are not broken.

Based on T92357: Fix problematic database master queries performed on HTTP GET/HEAD, there are still a few things that take actions based on GET requests (rollback, Special:Search), but to the best of my knowledge, all of those require tokens, so it can't be abused via URL shortener. There's some login related stuff which is irrelevant here, and T122708 which is just on any page view.

I'd be more concerned with random user scripts/gadgets having XSS holes...and those are security issues regardless of the existence of URL shortener or not.

for some reason Special:UserLogout as redirect target is blacklisted

It's for the same reason. It was blacklisted a long time ago.

Pppery added a subscriber: Pppery.Apr 14 2019, 9:05 PM

Special:UserLogout isn't blacklisted; the entire special namespace is

thiemowmde triaged this task as High priority.Apr 25 2019, 10:53 AM

@thiemowmde Why high when T25227: Use token when logging out is merged and will be deployed soon?