From 77910a621683bcd12325925dc5e15a73ab9114e3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Andr=C3=A9=20Noack?= <noack@data-quest.de>
Date: Thu, 21 Dec 2023 14:32:21 +0000
Subject: [PATCH] Resolve "OER-Campus: File upload vulnerabilities"

Closes #3586 and #3587

Merge request studip/studip!2480
---
 app/controllers/oer/endpoints.php     | 28 +++++++++++++++++++++------
 app/controllers/oer/market.php        | 15 ++++++++------
 app/controllers/oer/mymaterial.php    | 13 ++++++++++---
 app/controllers/oer/oai.php           |  7 +++++++
 app/controllers/studip_controller.php | 11 +++++++++--
 5 files changed, 57 insertions(+), 17 deletions(-)

diff --git a/app/controllers/oer/endpoints.php b/app/controllers/oer/endpoints.php
index cbd4b884593..417f1e8173f 100644
--- a/app/controllers/oer/endpoints.php
+++ b/app/controllers/oer/endpoints.php
@@ -328,6 +328,15 @@ class Oer_EndpointsController extends StudipController
             throw new Exception(_('Die gewünschte Datei konnte nicht gefunden werden.'));
         }
         $filesize = filesize($this->material->getFilePath());
+        $content_type = $this->material['content_type'];
+        if (!in_array($content_type, get_mime_types())) {
+            $content_type = 'application/octet-stream';
+        }
+        if ($content_type === 'application/octet-stream') {
+            $content_disposition = 'attachment';
+        } else {
+            $content_disposition = $disposition === 'attachment' ? 'attachment' : 'inline';
+        }
         header("Accept-Ranges: bytes");
         $start = 0;
         $end = $filesize - 1;
@@ -373,8 +382,8 @@ class Oer_EndpointsController extends StudipController
             header("Cache-Control: no-store, no-cache, must-revalidate");   // HTTP/1.1
         }
         header("Cache-Control: post-check=0, pre-check=0", false);
-        header("Content-Type: ".$this->material['content_type']);
-        header("Content-Disposition: " . ($disposition ?: "inline") . "; " . encode_header_parameter('filename', $this->material['filename']));
+        header("Content-Type: " . $content_type);
+        header("Content-Disposition: " . $content_disposition . "; " . encode_header_parameter('filename', $this->material['filename']));
 
         readfile_chunked($this->material->getFilePath(), $start, $end);
 
@@ -392,10 +401,17 @@ class Oer_EndpointsController extends StudipController
     public function download_front_image_action($material_id)
     {
         $this->material = new OERMaterial($material_id);
-        $this->set_content_type($this->material['front_image_content_type']);
-        $this->response->add_header('Content-Disposition', 'inline');
-        $this->response->add_header('Content-Length', filesize($this->material->getFrontImageFilePath()));
-        $this->render_text(file_get_contents($this->material->getFrontImageFilePath()));
+        if (
+            stripos($this->material['front_image_content_type'], 'image') === 0
+            && stripos($this->material['front_image_content_type'], 'svg') === false
+        ) {
+            $this->set_content_type($this->material['front_image_content_type']);
+            $this->response->add_header('Content-Disposition', 'inline');
+            $this->response->add_header('Content-Length', filesize($this->material->getFrontImageFilePath()));
+            $this->render_text(file_get_contents($this->material->getFrontImageFilePath()));
+        } else {
+            throw new Trails_Exception(404);
+        }
     }
 
     /**
diff --git a/app/controllers/oer/market.php b/app/controllers/oer/market.php
index 687551763f9..a80783ec2aa 100644
--- a/app/controllers/oer/market.php
+++ b/app/controllers/oer/market.php
@@ -8,6 +8,15 @@ class Oer_MarketController extends StudipController
     function before_filter(&$action, &$args)
     {
         parent::before_filter($action, $args);
+        if (
+            !Config::get()->OERCAMPUS_ENABLED
+            || (
+                Config::get()->OER_PUBLIC_STATUS !== 'nobody'
+                && !$GLOBALS['perm']->have_perm(Config::get()->OER_PUBLIC_STATUS)
+            )
+        ) {
+            throw new AccessDeniedException();
+        }
         Helpbar::Get()->addPlainText(
             _("Lernmaterialien"),
             _("Übungszettel, Musterlösungen, Vorlesungsmitschriften oder gar Folien, selbsterstellte Lernkarteikarten oder alte Klausuren. Das alles wird einmal erstellt und dann meist vergessen. Auf dem Lernmaterialienmarktplatz bleiben sie erhalten. Jeder kann was hochladen und jeder kann alles herunterladen. Alle Inhalte hier sind frei verfügbar für jeden.")
@@ -17,9 +26,6 @@ class Oer_MarketController extends StudipController
 
     public function index_action()
     {
-        if (!$GLOBALS['perm']->have_perm(Config::get()->OER_PUBLIC_STATUS)) {
-            throw new AccessDeniedException();
-        }
         if (Navigation::hasItem("/oer/market")) {
             Navigation::activateItem("/oer/market");
         }
@@ -153,9 +159,6 @@ class Oer_MarketController extends StudipController
 
     public function search_action()
     {
-        if (!$GLOBALS['perm']->have_perm(Config::get()->OER_PUBLIC_STATUS)) {
-            throw new AccessDeniedException();
-        }
         if (Navigation::hasItem("/oer/market")) {
             Navigation::activateItem("/oer/market");
         }
diff --git a/app/controllers/oer/mymaterial.php b/app/controllers/oer/mymaterial.php
index 27a949dd7f9..f29b2362402 100644
--- a/app/controllers/oer/mymaterial.php
+++ b/app/controllers/oer/mymaterial.php
@@ -7,6 +7,13 @@ class Oer_MymaterialController extends AuthenticatedController
     public function before_filter(&$action, &$args)
     {
         parent::before_filter($action, $args);
+        if (
+            !Config::get()->OERCAMPUS_ENABLED
+            ||
+            !$GLOBALS['perm']->have_perm(Config::get()->OER_PUBLIC_STATUS)
+        ) {
+            throw new AccessDeniedException();
+        }
         PageLayout::setTitle(_('Lernmaterialien'));
     }
 
@@ -47,7 +54,7 @@ class Oer_MymaterialController extends AuthenticatedController
             $material['host_id'] = null;
             $material['license_identifier'] = Request::get('license', 'CC-BY-SA-4.0');
             if (!empty($_FILES['file']['tmp_name'])) {
-                $material['content_type'] = $_FILES['file']['type'];
+                $material['content_type'] = get_mime_type($_FILES['file']['name']);
                 if (in_array($material['content_type'], $content_types)) {
                     mkdir($tmp_folder);
                     \Studip\ZipArchive::extractToPath($_FILES['file']['tmp_name'], $tmp_folder);
@@ -73,8 +80,8 @@ class Oer_MymaterialController extends AuthenticatedController
             }
 
 
-            if (!empty($_FILES['image']['tmp_name'])) {
-                $material['front_image_content_type'] = $_FILES['image']['type'];
+            if (!empty($_FILES['image']['tmp_name']) && is_array(getimagesize($_FILES['image']['tmp_name']))) {
+                $material['front_image_content_type'] = get_mime_type($_FILES['image']['name']);
                 move_uploaded_file($_FILES['image']['tmp_name'], $material->getFrontImageFilePath());
             } elseif (!empty($_SESSION['NEW_OER']['image_tmp_name'])) {
                 $material['front_image_content_type'] = get_mime_type($_SESSION['NEW_OER']['image_tmp_name']);
diff --git a/app/controllers/oer/oai.php b/app/controllers/oer/oai.php
index 7cc878646a3..d9f586e8f77 100644
--- a/app/controllers/oer/oai.php
+++ b/app/controllers/oer/oai.php
@@ -8,6 +8,13 @@
 class Oer_OaiController extends StudipController
 {
 
+    public function before_filter(&$action, &$args)
+    {
+        parent::before_filter($action, $args);
+        if (!Config::get()->OERCAMPUS_ENABLED) {
+            throw new AccessDeniedException();
+        }
+    }
     public function index_action()
     {
         $this->set_content_type('text/xml;charset=utf-8');
diff --git a/app/controllers/studip_controller.php b/app/controllers/studip_controller.php
index 1ddce67db50..7a4da54487c 100644
--- a/app/controllers/studip_controller.php
+++ b/app/controllers/studip_controller.php
@@ -504,8 +504,15 @@ abstract class StudipController extends Trails_Controller
         }
 
         if ($content_type === null) {
-            $finfo = finfo_open(FILEINFO_MIME_TYPE);
-            $content_type = finfo_file($finfo, $file);
+            $content_type = get_mime_type($filename ?: $file);
+        }
+
+        if (!in_array($content_type, get_mime_types())) {
+            $content_type = 'application/octet-stream';
+        }
+
+        if ($content_type === 'application/octet-stream') {
+            $content_disposition = 'attachment';
         }
 
         $this->set_content_type($content_type);
-- 
GitLab