From 6ce4044bfa85436bb89fab710c39ef7b28069386 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Wed, 25 Mar 2015 10:16:22 -0700
Subject: [PATCH] Lock MIME type configuration

Summary:
Ref T6755. This mitigates an attack where you:

  - compromise an administrative account;
  - configure "text/plain" as an "image" MIME type; and
  - create a new macro sourced from a sensitive resource which is locally accessible over HTTP GET, using DNS rebinding.

You can then view the content of the resource in Files. By preventing the compromised account from reconfiguring the MIME types, the server will instead destroy the response and prevent the attacker from seeing it.

In general, these options should change very rarely, and they often sit just beyond the edge of security vulnerabilities anyway.

For example, if you ignore the warnings about an alternate file domain and elect to serve content from the primary domain, it's still somewhat difficult for an attacker to exploit the vulnerability. If they can add "text/html" or "image/svg+xml" as image MIME types, it becomes trivial. In this case not having an alternate domain is the main issue, but easy modification of this config increases risk/exposure.

Test Plan: Viewed affected config and saw that it is locked.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T6755

Differential Revision: https://secure.phabricator.com/D12154
---
 .../files/config/PhabricatorFilesConfigOptions.php       | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/src/applications/files/config/PhabricatorFilesConfigOptions.php b/src/applications/files/config/PhabricatorFilesConfigOptions.php
index b448b0b5ff..7f18da3e99 100644
--- a/src/applications/files/config/PhabricatorFilesConfigOptions.php
+++ b/src/applications/files/config/PhabricatorFilesConfigOptions.php
@@ -89,8 +89,14 @@ final class PhabricatorFilesConfigOptions
 
     ) + array_fill_keys(array_keys($image_default), 'fa-file-image-o');
 
+    // NOTE: These options are locked primarily because adding "text/plain"
+    // as an image MIME type increases SSRF vulnerability by allowing users
+    // to load text files from remote servers as "images" (see T6755 for
+    // discussion).
+
     return array(
       $this->newOption('files.viewable-mime-types', 'wild', $viewable_default)
+        ->setLocked(true)
         ->setSummary(
           pht('Configure which MIME types are viewable in the browser.'))
         ->setDescription(
@@ -104,18 +110,21 @@ final class PhabricatorFilesConfigOptions
             'the MIME types they are delivered as when they are viewed in '.
             'the browser.')),
       $this->newOption('files.image-mime-types', 'set', $image_default)
+        ->setLocked(true)
         ->setSummary(pht('Configure which MIME types are images.'))
         ->setDescription(
           pht(
             'List of MIME types which can be used as the `src` for an '.
             '`<img />` tag.')),
       $this->newOption('files.audio-mime-types', 'set', $audio_default)
+        ->setLocked(true)
         ->setSummary(pht('Configure which MIME types are audio.'))
         ->setDescription(
           pht(
             'List of MIME types which can be used to render an '.
             '`<audio />` tag.')),
       $this->newOption('files.icon-mime-types', 'wild', $icon_default)
+        ->setLocked(true)
         ->setSummary(pht('Configure which MIME types map to which icons.'))
         ->setDescription(
           pht(