Page MenuHomePhabricator

Replace "upload by url" allow-list by deny-list
Open, LowestPublicFeature

Description

There is no reason to maintain a "upload by url" whitelist instead of a blacklist considering that there should not be any technical difficulties (except bug 42473 ).

Furthermore only trusted community members (e.g. administrators) are granted the "upload_by_url" right so this is not a problem as well.


Version: 1.23.0
Severity: enhancement
See Also:
T47735: Allow other domains in $wgCopyUploadsDomains to enable full support of upload_by_url

Details

Reference
bz63961

Event Timeline

bzimport raised the priority of this task from to Lowest.Nov 22 2014, 3:07 AM
bzimport set Reference to bz63961.
bzimport added a subscriber: Unknown Object (MLST).

Apparently opened in response to my request for justification on https://gerrit.wikimedia.org/r/#/c/126384/ (Adding '*.panoramio.com' to the wgCopyUploadsDomains array).

(In reply to Marco from comment #0)

There is no reason to maintain a "upload by url" whitelist instead of a
blacklist considering that there should not be any technical difficulties
(except bug 42473 ).

Why?

If we're going to whitelist all, there's probably little point attempting to blacklist anything

(In reply to Sam Reed (reedy) from comment #2)

Why?

  • Do you mean that some domains could serve corrupt files which can compromise the wmf network when uploading by url?
  • Did we encounter any problems after adding the ~20 urls we currently have in the commonswiki-array?
  • Is there any other way to find out if there are problems than whitelisting all domains in a test environment?

If we're going to whitelist all, there's probably little point attempting to
blacklist anything

Thats true, would be "nice" to have, though.

(In reply to Marco from comment #3)

(In reply to Sam Reed (reedy) from comment #2)

Why?

  • Do you mean that some domains could serve corrupt files which can

compromise the wmf network when uploading by url?

I believe the concern is potential to be used as part of a DOS attack. Invalid files/viruses/etc are not really a concern as that is not unique to url uploading.

  • Did we encounter any problems after adding the ~20 urls we currently have

in the commonswiki-array?

Not as far as I know. I highly doubt it.

  • Is there any other way to find out if there are problems than whitelisting

all domains in a test environment?

Any issues would probably be for security reasons (or perhaps patanoia) i believe. Thats not something a test site would help with. Its the sort of thing that needs to be analytically evaluated (by Chris?)

Of course i could just be missing some big issue.

If we're going to whitelist all, there's probably little point attempting to
blacklist anything

Thats true, would be "nice" to have, though.

I dont see why. Does anyone actually have any sites to blacklist?.


The why for this is presumably commons folks want to be able to use gwtoolset with new sites without asking for a config change first (and having to wait several days). I could certainly see why - instant gratification is more fun :)

I prefer a whitelist for two reasons:

  • When we have security issues that effect the outbound connection (the curl-imap overflow, and need I even bring up heartbleed), then forcing a delay between when someone setting up a hostile server to exploit the flow and getting requests from wmf servers is a good thing.
  • If an attacker is running a 0-day attack, and gets the url approved before we patch our servers, we at least have an audit log of who requested and approved the url whenever we figure out it's hostile.

(In reply to Chris Steipp from comment #5)

then forcing a delay is a good thing

That's true. And what I notice is that none of the "big" sites allow users upload-by-url.
On the other hand you'll have to spam the config more and more and there's always someone required to merge the patch. These people are usually short in time.

we at least have an audit log of who requested and approved the url whenever we
figure out it's hostile

I suppose you cannot blame the approver as a site may look completely harmless when looking at it and as for the requester/patchset uploader, you do not require personal identification so you do not have more information compared to which user issued the action=upload&url=... command at Commons.

Yann raised the priority of this task from Lowest to Medium.Mar 26 2015, 10:36 AM
Yann set Security to None.
Dereckson lowered the priority of this task from Medium to Lowest.Nov 13 2015, 2:07 PM
Dereckson subscribed.

Resetting priority per Reedy comment about the interest of a blacklist if we remove the whitelist requirement.

Peachey88 renamed this task from Replace "upload by url" whitelist by blacklist to Replace "upload by url" allow-list by deny-list.Jan 30 2022, 4:29 AM
Peachey88 updated the task description. (Show Details)
Aklapper changed the subtype of this task from "Task" to "Feature Request".Feb 4 2022, 11:14 AM