Page MenuHomePhabricator

D19421.diff
No OneTemporary

D19421.diff

diff --git a/src/applications/files/controller/PhabricatorFileDataController.php b/src/applications/files/controller/PhabricatorFileDataController.php
--- a/src/applications/files/controller/PhabricatorFileDataController.php
+++ b/src/applications/files/controller/PhabricatorFileDataController.php
@@ -84,12 +84,29 @@
$response->setMimeType($file->getViewableMimeType());
} else {
$is_post = $request->isHTTPPost();
+ $is_public = !$viewer->isLoggedIn();
// NOTE: Require POST to download files from the primary domain. If the
// request is not a POST request but arrives on the primary domain, we
// render a confirmation dialog. For discussion, see T13094.
- $is_safe = ($is_alternate_domain || $is_lfs || $is_post);
+ // There are two exceptions to this rule:
+
+ // Git LFS requests can download with GET. This is safe (Git LFS won't
+ // execute files it downloads) and necessary to support Git LFS.
+
+ // Requests with no credentials may also download with GET. This
+ // primarily supports downloading files with `arc download` or other
+ // API clients. This is only "mostly" safe: if you aren't logged in, you
+ // are likely immune to XSS and CSRF. However, an attacker may still be
+ // able to set cookies on this domain (for example, to fixate your
+ // session). For now, we accept these risks because users running
+ // Phabricator in this mode are knowingly accepting a security risk
+ // against setup advice, and there's significant value in having
+ // API development against test and production installs work the same
+ // way.
+
+ $is_safe = ($is_alternate_domain || $is_post || $is_lfs || $is_public);
if (!$is_safe) {
return $this->newDialog()
->setSubmitURI($file->getDownloadURI())

File Metadata

Mime Type
text/plain
Expires
Wed, Mar 5, 9:43 AM (2 w, 1 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7224801
Default Alt Text
D19421.diff (1 KB)

Event Timeline