Page MenuHomePhabricator

D17613.diff
No OneTemporary

D17613.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,18 +84,28 @@
if ($is_viewable && !$force_download) {
$response->setMimeType($file->getViewableMimeType());
} else {
- if (!$request->isHTTPPost() && !$is_alternate_domain && !$is_lfs) {
- // NOTE: Require POST to download files from the primary domain. We'd
- // rather go full-bore and do a real CSRF check, but can't currently
- // authenticate users on the file domain. This should blunt any
- // attacks based on iframes, script tags, applet tags, etc., at least.
- // Send the user to the "info" page if they're using some other method.
+ $is_public = !$viewer->isLoggedIn();
+ $is_post = $request->isHTTPPost();
+ // NOTE: Require POST to download files from the primary domain if the
+ // request includes credentials. The "Download File" links we generate
+ // in the web UI are forms which use POST to satisfy this requirement.
+
+ // The intent is to make attacks based on tags like "<iframe />" and
+ // "<script />" (which can issue GET requests, but can not easily issue
+ // POST requests) more difficult to execute.
+
+ // The best defense against these attacks is to use an alternate file
+ // domain, which is why we strongly recommend doing so.
+
+ $is_safe = ($is_alternate_domain || $is_lfs || $is_post || $is_public);
+ if (!$is_safe) {
// This is marked as "external" because it is fully qualified.
return id(new AphrontRedirectResponse())
->setIsExternal(true)
->setURI(PhabricatorEnv::getProductionURI($file->getBestURI()));
}
+
$response->setMimeType($file->getMimeType());
$response->setDownload($file->getName());
}

File Metadata

Mime Type
text/plain
Expires
Tue, Oct 7, 3:22 PM (3 w, 5 h ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
9378367
Default Alt Text
D17613.diff (1 KB)

Event Timeline