Attachment Security Update

.gitignore

@@ -31,10 +31,10 @@ www.pid
 *.swo
 
 # Attachments directory
-src/data/attachments/*
-!src/data/attachments/index.php
-src/data/attachments/deleted/*
-!src/data/attachments/deleted/index.php
+attachments/*
+!attachments/index.php
+attachments/deleted/*
+!attachments/deleted/index.php
 
 # Custom logos directory
 src/data/customlogos/*

extra/nginx/nginx.conf

@@ -38,12 +38,6 @@ server {
   root CTFPATH;
   index index.php;
 
-  location /data/attachments/ {
-    fastcgi_param SCRIPT_FILENAME $document_root$fastcgi_script_name;
-    include fastcgi_params;
-    fastcgi_pass HHVMSERVER:9000;
-  }
-
   location /data/customlogos/ {
     fastcgi_param SCRIPT_FILENAME $document_root$fastcgi_script_name;
     include fastcgi_params;

extra/provision.sh

@@ -326,10 +326,10 @@ fi
     fi
 
     log "Creating attachments folder, and setting ownership to www-data"
-    sudo sudo mkdir -p "$CTF_PATH/src/data/attachments"
-    sudo sudo mkdir -p "$CTF_PATH/src/data/attachments/deleted"
-    sudo chown -R www-data:www-data "$CTF_PATH/src/data/attachments"
-    sudo chown -R www-data:www-data "$CTF_PATH/src/data/attachments/deleted"
+    sudo sudo mkdir -p "$CTF_PATH/attachments"
+    sudo sudo mkdir -p "$CTF_PATH/attachments/deleted"
+    sudo chown -R www-data:www-data "$CTF_PATH/attachments"
+    sudo chown -R www-data:www-data "$CTF_PATH/attachments/deleted"
 
     log "Creating custom logos folder, and setting ownership to www-data"
     sudo mkdir -p "$CTF_PATH/src/data/customlogos"

src/controllers/AdminController.php

@@ -4,7 +4,7 @@ class AdminController extends Controller {
   <<__Override>>
   protected function getTitle(): string {
     $custom_org = \HH\Asio\join(Configuration::gen('custom_org'));
-    return tr($custom_org->getValue()). ' '. tr('CTF'). ' | '. tr('Admin');
+    return tr($custom_org->getValue()).' '.tr('CTF').' | '.tr('Admin');
   }
 
   <<__Override>>
@@ -157,8 +157,8 @@ class="fb--conf--registration_type"
     $select = <select name="fb--conf--password_type"></select>;
     foreach ($types as $type) {
       $select->appendChild(
-        <option 
-          class="fb--conf--password_type" 
+        <option
+          class="fb--conf--password_type"
           value={strval($type->getField())}
           selected={($type->getField() === $config->getField())}>
           {$type->getDescription()}
@@ -476,15 +476,15 @@ class="fb-cta cta--yellow"
       $custom_logo_xhp =
         <div class="form-el el--block-label el--full-text">
           <label for="">{tr('Logo')}</label>
-          <img 
-            id="custom-logo-image" 
-            class="icon--badge" 
+          <img
+            id="custom-logo-image"
+            class="icon--badge"
             src={$custom_logo_image->getValue()}
           />
-          <br/>
-           <h6>
+          <br />
+          <h6>
             <a class="icon-text" href="#" id="custom-logo-link">
-            {tr('Change')}
+              {tr('Change')}
             </a>
           </h6>
           <input
@@ -2014,7 +2014,7 @@ class="fb-cta cta--yellow"
                         value={$attachment->getFilename()}
                         disabled={true}
                       />
-                      <a href={$attachment->getFilename()} target="_blank">
+                      <a href={$attachment->getFileLink()} target="_blank">
                         {tr('Link')}
                       </a>
                     </div>
@@ -2547,7 +2547,7 @@ class="fb-cta cta--yellow"
                         value={$attachment->getFilename()}
                         disabled={true}
                       />
-                      <a href={$attachment->getFilename()} target="_blank">
+                      <a href={$attachment->getFileLink()} target="_blank">
                         {tr('Link')}
                       </a>
                     </div>

src/data/attachment.php

@@ -0,0 +1,41 @@
+<?hh // strict
+
+require_once ($_SERVER['DOCUMENT_ROOT'].'/../vendor/autoload.php');
+
+class AttachmentDataController extends DataController {
+  public async function genGenerateData(): Awaitable<void> {
+
+    /* HH_IGNORE_ERROR[1002] */
+    SessionUtils::sessionStart();
+    SessionUtils::enforceLogin();
+
+    await tr_start();
+
+    $data = tr('File Does Not Exist');
+    $filename = tr('error');
+
+    $attachment_id = idx(Utils::getGET(), 'id', '');
+    if (intval($attachment_id) !== 0) {
+      $attachment_exists =
+        await Attachment::genCheckExists(intval($attachment_id));
+      if ($attachment_exists === true) {
+        $attachment = await Attachment::gen(intval($attachment_id));
+        $filename = $attachment->getFilename();
+
+        // Remove all non alpahnum characters from filename - allow international chars, dash, underscore, and period
+        $filename = preg_replace('/[^\p{L}\p{N}_\-.]+/u', '_', $filename);
+
+        $data = readfile(Attachment::attachmentsDir.$filename);
+      }
+    }
+
+    header('Content-Type: application/octet-stream');
+    header("Content-Transfer-Encoding: Binary");
+    header('Content-disposition: attachment; filename="'.$filename.'"');
+    print $data;
+  }
+}
+
+/* HH_IGNORE_ERROR[1002] */
+$attachment_file = new AttachmentDataController();
+\HH\Asio\join($attachment_file->genGenerateData());

src/data/country-data.php

@@ -63,7 +63,10 @@ class CountryDataController extends DataController {
         $all_attachments =
           await Attachment::genAllAttachments($level->getId());
         foreach ($all_attachments as $attachment) {
-          array_push($attachments_list, $attachment->getFilename());
+          $attachment_details = array();
+          $attachment_details['filename'] = $attachment->getFilename();
+          $attachment_details['file_link'] = $attachment->getFileLink();
+          array_push($attachments_list, $attachment_details);
         }
       }
 

src/models/Attachment.php

@@ -2,7 +2,7 @@
 
 class Attachment extends Model {
   // TODO: Configure this
-  const string attachmentsDir = '/data/attachments/';
+  const string attachmentsDir = '/var/www/fbctf/attachments/';
 
   protected static string $MC_KEY = 'attachments:';
 
@@ -17,6 +17,7 @@ private function __construct(
     private int $id,
     private int $levelId,
     private string $filename,
+    private string $link,
     private string $type,
   ) {}
 
@@ -28,6 +29,10 @@ public function getFilename(): string {
     return $this->filename;
   }
 
+  public function getFileLink(): string {
+    return $this->link;
+  }
+
   public function getType(): string {
     return $this->type;
   }
@@ -44,7 +49,8 @@ public function getLevelId(): int {
   ): Awaitable<bool> {
     $db = await self::genDb();
     $type = '';
-    $local_filename = self::attachmentsDir;
+    $file_path = self::attachmentsDir;
+    $local_filename = '';
 
     $files = Utils::getFILES();
     $server = Utils::getSERVER();
@@ -63,30 +69,21 @@ public function getLevelId(): int {
         $local_filename .= '.'.$extension;
       }
 
-      // Avoid php shells
-      if (ends_with($local_filename, '.php')) {
-        $local_filename .= 's'; // Make the extension 'phps'
-      }
-      move_uploaded_file(
-        $tmp_name,
-        must_have_string($server, 'DOCUMENT_ROOT').$local_filename,
-      );
+      // Remove all non alpahnum characters from filename - allow international chars, dash, underscore, and period
+      $local_filename =
+        preg_replace('/[^\p{L}\p{N}_\-.]+/u', '_', $local_filename);
+
+      move_uploaded_file($tmp_name, $file_path.$local_filename);
 
       // Force 0600 Permissions
-      $chmod = chmod(
-        must_have_string($server, 'DOCUMENT_ROOT').$local_filename,
-        0600,
-      );
+      $chmod = chmod($file_path.$local_filename, 0600);
       invariant(
         $chmod === true,
         'Failed to set attachment file permissions to 0600',
       );
 
       // Force ownership to www-data
-      $chown = chown(
-        must_have_string($server, 'DOCUMENT_ROOT').$local_filename,
-        'www-data',
-      );
+      $chown = chown($file_path.$local_filename, 'www-data');
       invariant(
         $chown === true,
         'Failed to set attachment file ownership to www-data',
@@ -132,7 +129,7 @@ public function getLevelId(): int {
 
     // Copy file to deleted folder
     $attachment = await self::gen($attachment_id);
-    $filename = $attachment->getFilename();
+    $filename = self::attachmentsDir.$attachment->getFilename();
     $parts = pathinfo($filename);
     error_log(
       'Copying from '.
@@ -142,9 +139,8 @@ public function getLevelId(): int {
       '/deleted/'.
       $parts['basename'],
     );
-    $root = strval($server['DOCUMENT_ROOT']);
-    $origin = $root.$filename;
-    $dest = $root.$parts['dirname'].'/deleted/'.$parts['basename'];
+    $origin = $filename;
+    $dest = $parts['dirname'].'/deleted/'.$parts['basename'];
     copy($origin, $dest);
 
     // Delete file.
@@ -242,6 +238,31 @@ public function getLevelId(): int {
     }
   }
 
+  public static async function genCheckExists(
+    int $attachment_id,
+    bool $refresh = false,
+  ): Awaitable<bool> {
+    $mc_result = self::getMCRecords('ATTACHMENTS');
+    if (!$mc_result || count($mc_result) === 0 || $refresh) {
+      $db = await self::genDb();
+      $attachments = Map {};
+      $result = await $db->queryf('SELECT * FROM attachments');
+      foreach ($result->mapRows() as $row) {
+        $attachments->add(
+          Pair {intval($row->get('id')), self::attachmentFromRow($row)},
+        );
+      }
+      self::setMCRecords('ATTACHMENTS', $attachments);
+      return $attachments->contains($attachment_id);
+    } else {
+      invariant(
+        $mc_result instanceof Map,
+        'cache return should be of type Map',
+      );
+      return $mc_result->contains($attachment_id);
+    }
+  }
+
   // Check if a level has attachments.
   public static async function genHasAttachments(
     int $level_id,
@@ -304,6 +325,7 @@ private static function attachmentFromRow(
       intval(must_have_idx($row, 'id')),
       intval(must_have_idx($row, 'level_id')),
       must_have_idx($row, 'filename'),
+      strval('/data/attachment.php?id='.intval(must_have_idx($row, 'id'))),
       must_have_idx($row, 'type'),
     );
   }

src/models/Control.php

@@ -422,14 +422,17 @@ class Control extends Model {
     $filename =
       strval(BinaryImporterController::getFilename('attachments_file'));
     $document_root = must_have_string(Utils::getSERVER(), 'DOCUMENT_ROOT');
-    $directory = $document_root.Attachment::attachmentsDir;
-    $cmd = "tar -zx -C $directory -f $filename";
+    $directory = Attachment::attachmentsDir;
+    $cmd = "tar -zx --mode=600 -C $directory -f $filename";
     exec($cmd, $output, $status);
     if (intval($status) !== 0) {
       return false;
     }
-    $directory_files = scandir($directory);
+    $directory_files = array_slice(scandir($directory), 2);
     foreach ($directory_files as $file) {
+      if (is_dir($file) === true) {
+        continue;
+      }
       $chmod = chmod($directory.$file, 0600);
       invariant(
         $chmod === true,
@@ -507,7 +510,7 @@ class Control extends Model {
     header('Content-Type: application/x-tgz');
     header('Content-Disposition: attachment; filename="'.$filename.'"');
     $document_root = must_have_string(Utils::getSERVER(), 'DOCUMENT_ROOT');
-    $directory = $document_root.Attachment::attachmentsDir;
+    $directory = Attachment::attachmentsDir;
     $cmd = "tar -cz -C $directory . ";
     passthru($cmd);
     exit();

src/static/js/fb-ctf.js

@@ -934,8 +934,10 @@ function setupInputListeners() {
         $('.capture-text', $container).text(intro);
         if (attachments instanceof Array) {
           $.each(attachments, function() {
-            var f = this.substr(this.lastIndexOf('/') + 1);
-            var attachment = $('<a/>').attr('target', '_blank').attr('href', this).text('[ ' + f + ' ]');
+            var filename = this['filename'];
+            var link = this['file_link'];
+            var f = filename.substr(filename.lastIndexOf('/') + 1);
+            var attachment = $('<a/>').attr('target', '_blank').attr('href', link).text('[ ' + f + ' ]');
             $('.capture-links', $container).append(attachment);
             $('.capture-links', $container).append($('<br/>'));
           });