Page MenuHomePhabricator

UrlShortener extension accepts arbitrary port numbers and credentials
Closed, ResolvedPublic

Description

e.g., "https://en.wikipedia.org:22" and "https://user:password@graphite.wikimedia.org" both work (the URL is shortened and the target URL preserves the port number and authentication credentials).

Since URI schemes are normalized to https / http, restricting port to ports 80 and 443 seems like a good idea. Auth credentials should be stripped or rejected as invalid input.

Event Timeline

ori created this task.Aug 10 2015, 5:54 PM
ori raised the priority of this task from to Normal.
ori updated the task description. (Show Details)
ori added a subscriber: Legoktm.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptAug 10 2015, 5:54 PM
ori renamed this task from UrlShortener extension accepts arbitrary port numbers to UrlShortener extension accepts arbitrary port numbers and credentials.Aug 10 2015, 5:58 PM
ori updated the task description. (Show Details)
ori set Security to None.

Is there a valid reason to include port 80/443 in URLs? We still need to support port numbers that may be in $wgServer (like in mw-vagrant for example), so might be easiest to implement this in the regex...

Auth credentials should be easy to reject since both wfParseUrl() and mw.Uri expose those.

ori added a subscriber: ori.Aug 10 2015, 6:57 PM

Is there a valid reason to include port 80/443 in URLs?

No. Stripping port numbers outright (or rejecting URLs which contain them) would be fine, I think.

Change 230988 had a related patch set uploaded (by Legoktm):
Disallow URLs that contain ports that aren't 80 or 443 by default

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

Change 230988 merged by jenkins-bot:
Disallow URLs that contain ports that aren't 80 or 443 by default

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

ori closed this task as Resolved.Aug 13 2015, 5:19 PM
ori claimed this task.
Legoktm reopened this task as Open.Aug 13 2015, 5:33 PM

We didn't fix username/password yet, just arbitrary ports.

Change 231744 had a related patch set uploaded (by Legoktm):
Don't allow shortening URLs with a username or password

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

Change 231744 merged by jenkins-bot:
Don't allow shortening URLs with a username or password

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

Legoktm closed this task as Resolved.Aug 16 2015, 10:42 PM