From 37b93f426225280f24fd2aaf2109576a19896398 Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 7 Apr 2016 08:45:56 -0700 Subject: [PATCH] Don't require POST to download LFS files from main domain Summary: Ref T7789. If you don't have `security.alternate-file-domain` configured, we won't serve binary files over GET. This is a security measure intended to prevent `` attacks and similar, where you upload some "dangerous" binary, include it in another page, and it gets some of the host's permissions because Java/Flash security models are (or were, in the past) goofy. Allow them to be served over GET if the client is Git LFS. This is safe; these attacks can't add arbitrary HTTP headers. Test Plan: Fetched files over GET with and without the LFS header. ``` $ curl -v http://local.phacility.com/file/data/@local/jfht2cxjazi5cmjomfhl/PHID-FILE-sa7mh2pfaocz2adiimeh/netgear_rma.pdf > /dev/null ... HTTP 302 Redirect ... ``` ``` $ curl -v -H 'X-Phabricator-Request-Type: git-lfs' http://localcontent.phacility.com/file/data/@local/jfht2cxjazi5cmjomfhl/PHID-FILE-sa7mh2pfaocz2adiimeh/netgear_rma.pdf > /dev/null ... HTTP 200 Content ... ``` Reviewers: chad Reviewed By: chad Maniphest Tasks: T7789 Differential Revision: https://secure.phabricator.com/D15654 --- .../diffusion/controller/DiffusionServeController.php | 1 + .../files/controller/PhabricatorFileDataController.php | 5 ++++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/applications/diffusion/controller/DiffusionServeController.php b/src/applications/diffusion/controller/DiffusionServeController.php index fab2bcca96..a5871ca074 100644 --- a/src/applications/diffusion/controller/DiffusionServeController.php +++ b/src/applications/diffusion/controller/DiffusionServeController.php @@ -996,6 +996,7 @@ final class DiffusionServeController extends DiffusionController { 'href' => $get_uri, 'header' => array( 'Authorization' => $no_authorization, + 'X-Phabricator-Request-Type' => 'git-lfs', ), ); } else { diff --git a/src/applications/files/controller/PhabricatorFileDataController.php b/src/applications/files/controller/PhabricatorFileDataController.php index e7ff2827e5..dfae99d201 100644 --- a/src/applications/files/controller/PhabricatorFileDataController.php +++ b/src/applications/files/controller/PhabricatorFileDataController.php @@ -82,10 +82,13 @@ final class PhabricatorFileDataController extends PhabricatorFileController { $is_viewable = $file->isViewableInBrowser(); $force_download = $request->getExists('download'); + $request_type = $request->getHTTPHeader('X-Phabricator-Request-Type'); + $is_lfs = ($request_type == 'git-lfs'); + if ($is_viewable && !$force_download) { $response->setMimeType($file->getViewableMimeType()); } else { - if (!$request->isHTTPPost() && !$is_alternate_domain) { + 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