Bug 1834124 - Stored XSS via file sharer

Author: dklawren

src/applications/files/config/PhabricatorFilesConfigOptions.php

@@ -34,19 +34,23 @@ public function getOptions() {
       'image/x-icon'              => 'image/x-icon',
       'image/vnd.microsoft.icon'  => 'image/x-icon',
 
+      // Commented out the below mimetypes for security/privacy concerns.
+      // It just means these will need to be downloaded instead of viewable
+      // in the page.
+      
       // This is a generic type for both OGG video and OGG audio.
-      'application/ogg' => 'application/ogg',
+      // 'application/ogg' => 'application/ogg',
 
-      'audio/x-wav' => 'audio/x-wav',
-      'audio/mpeg' => 'audio/mpeg',
-      'audio/ogg' => 'audio/ogg',
+      // 'audio/x-wav' => 'audio/x-wav',
+      // 'audio/mpeg' => 'audio/mpeg',
+      // 'audio/ogg' => 'audio/ogg',
 
-      'video/mp4' => 'video/mp4',
-      'video/ogg' => 'video/ogg',
-      'video/webm' => 'video/webm',
-      'video/quicktime' => 'video/quicktime',
+      // 'video/mp4' => 'video/mp4',
+      // 'video/ogg' => 'video/ogg',
+      // 'video/webm' => 'video/webm',
+      // 'video/quicktime' => 'video/quicktime',
 
-      'application/pdf' => 'application/pdf',
+      // 'application/pdf' => 'application/pdf',
     );
 
     $image_default = array(

src/applications/files/document/PhabricatorDocumentEngine.php

@@ -30,6 +30,12 @@ final public function getHighlightedLines() {
   final public function canRenderDocument(PhabricatorDocumentRef $ref) {
     return $this->canRenderDocumentType($ref);
   }
+ 
+  protected function canRenderDocumentType(PhabricatorDocumentRef $ref) {
+    $viewable_types = PhabricatorEnv::getEnvConfig('files.viewable-mime-types');
+    $viewable_types = array_keys($viewable_types);
+    return $ref->hasAnyMimeType($viewable_types);
+  }
 
   public function canDiffDocuments(
     PhabricatorDocumentRef $uref = null,
@@ -114,9 +120,6 @@ public function shouldRenderAsync(PhabricatorDocumentRef $ref) {
     return false;
   }
 
-  abstract protected function canRenderDocumentType(
-    PhabricatorDocumentRef $ref);
-
   final public function newDocument(PhabricatorDocumentRef $ref) {
     $can_complete = $this->canRenderCompleteDocument($ref);
     $can_partial = $this->canRenderPartialDocument($ref);

src/applications/files/document/PhabricatorPDFDocumentEngine.php

@@ -13,16 +13,21 @@ protected function getDocumentIconIcon(PhabricatorDocumentRef $ref) {
     return 'fa-file-pdf-o';
   }
 
-  protected function canRenderDocumentType(PhabricatorDocumentRef $ref) {
-    // Since we just render a link to the document anyway, we don't need to
-    // check anything fancy in config to see if the MIME type is actually
-    // viewable.
-
-    return $ref->hasAnyMimeType(
-      array(
-        'application/pdf',
-      ));
-  }
+  /* MOZILLA: Instead of always allowing PDFs to be viewable in the web UI,
+     we use the base canRenderDocumentType() from PhabricatorDocumentEngine.
+     This checks that the PDF mimetype is viewable according to the system
+     configuration. */
+  
+  // protected function canRenderDocumentType(PhabricatorDocumentRef $ref) {
+  //   // Since we just render a link to the document anyway, we don't need to
+  //   // check anything fancy in config to see if the MIME type is actually
+  //   // viewable.
+
+  //   return $ref->hasAnyMimeType(
+  //     array(
+  //       'application/pdf',
+  //     ));
+  // }
 
   protected function newDocumentContent(PhabricatorDocumentRef $ref) {
     $viewer = $this->getViewer();