Page MenuHomePhabricator

Restore support for using "arc download" to fetch files with no "security.alternate-file-domain"
ClosedPublic

Authored by epriestley on May 1 2018, 2:21 PM.
Tags
None
Referenced Files
F14082571: D19421.diff
Sat, Nov 23, 12:24 AM
Unknown Object (File)
Thu, Nov 7, 10:31 AM
Unknown Object (File)
Thu, Nov 7, 10:31 AM
Unknown Object (File)
Wed, Nov 6, 12:13 AM
Unknown Object (File)
Wed, Nov 6, 12:13 AM
Unknown Object (File)
Wed, Nov 6, 12:13 AM
Unknown Object (File)
Mon, Nov 4, 10:37 AM
Unknown Object (File)
Sun, Oct 27, 9:14 PM
Subscribers
None

Details

Summary

Fixes T13132. I removed this branch in D19156 when tightening the logic for the new CSP header, but there's a legitimate need for it: downloading files via arc download, or more generally being an API consumer of files.

This is not completely safe, but attacks I'm aware of (particularly, cookie fixation, where an attacker could potentially force a victim to become logged in to an account they control) are difficult and not very powerful. We already issue clear setup advice about the importance of configuring this option ("Phabricator is currently configured to serve user uploads directly from the same domain as other content. This is a security risk.") and I think there's significant value in letting API clients just GET file data without having to jump through a lot of weird hoops.

Test Plan
  • With security.alternate-file-domain off, tried to arc download a file.
  • Before: downloaded an HTML dialog page.
  • After: downloaded the file.

Diff Detail

Repository
rP Phabricator
Branch
download1
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 20268
Build 27512: Run Core Tests
Build 27511: arc lint + arc unit

Event Timeline

The attack is something like this:

  • You upload a .html page to Phabricator containing some malicious Javascript.
  • You somehow get your victim to visit the page and execute the HTML in their browser while logged out. This is normally difficult because we try very hard to prevent this, but not necessarily impossible since browsers tend to want to execute HTML and Javascript. For example, earlier versions of Safari on iPad ignored Content-Disposition: attachment and would execute (instead of downloading) HTML files.

When the user visits and executes your page:

  • You set their phsid (session) cookie to your session cookie.
  • On the server, Phabricator can't distinguish between the valid phsid cookie it set and the copy of the phsid cookie your evil page added, so this is treated as a valid phsid cookie and your victim is now logged into an account you control.
  • Through social engineering, you now somehow get them to enter something super secret into Phabricator, not noticing that you've logged them in as you and believing they are entering the data into their own account.
  • Since you control the account they're actually logged into, you can later go retrieve the data?

I think this is an acceptable risk since:

  • Getting the victim to execute the script is hard.
  • Getting them to enter secret data and not notice that they're logged in as someone else also seems hard.
  • This attack is completely defused by following setup advice and configuring an alternate file domain.
  • This attack might be possible anyway, whether we allow these public GET requests or not, depending on how you're tricking the script into executing.
This revision is now accepted and ready to land.May 1 2018, 4:26 PM
This revision was automatically updated to reflect the committed changes.