From 46e07eb8ceddfb3fbe4fadc81a56f42f1932739f Mon Sep 17 00:00:00 2001
From: Nick <nick@fauchelle.com>
Date: Mon, 8 Jan 2024 15:49:51 +1300
Subject: [PATCH] FIX Ensure the setAllowedMaxFileSize wont allow config values
 to exceed PHP limits (#469)

Fixes an issue where you could specify a upload validation size larger than the php in allowed value. This will now check to ensure what is set on config wont exeed php limit. This also uses Convert since File::ini2bytes is depricated, added some tests
---
 src/Upload_Validator.php | 51 +++++++++++++++++++++++----------
 tests/php/UploadTest.php | 61 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 97 insertions(+), 15 deletions(-)

diff --git a/src/Upload_Validator.php b/src/Upload_Validator.php
index 774ec87e..50649c13 100644
--- a/src/Upload_Validator.php
+++ b/src/Upload_Validator.php
@@ -122,7 +122,6 @@ public function getLargestAllowedMaxFileSize()
      */
     public function getAllowedMaxFileSize($ext = null)
     {
-
         // Check if there is any defined instance max file sizes
         if (empty($this->allowedMaxFileSize)) {
             // Set default max file sizes if there isn't
@@ -131,9 +130,7 @@ public function getAllowedMaxFileSize($ext = null)
                 $this->setAllowedMaxFileSize($fileSize);
             } else {
                 // When no default is present, use maximum set by PHP
-                $maxUpload = Convert::memstring2bytes(ini_get('upload_max_filesize'));
-                $maxPost = Convert::memstring2bytes(ini_get('post_max_size'));
-                $this->setAllowedMaxFileSize(min($maxUpload, $maxPost));
+                $this->setAllowedMaxFileSize($this->maxUploadSizeFromPHPIni());
             }
         }
 
@@ -174,25 +171,49 @@ public function setAllowedMaxFileSize($rules)
             $finalRules = [];
 
             foreach ($rules as $rule => $value) {
-                if (is_numeric($value)) {
-                    $tmpSize = $value;
-                } else {
-                    $tmpSize = Convert::memstring2bytes($value);
-                }
-
-                $finalRules[$rule] = (int)$tmpSize;
+                $finalRules[$rule] = $this->parseAndVerifyAllowedSize($value);
             }
 
             $this->allowedMaxFileSize = $finalRules;
-        } elseif (is_string($rules)) {
-            $this->allowedMaxFileSize['*'] = Convert::memstring2bytes($rules);
-        } elseif ((int)$rules > 0) {
-            $this->allowedMaxFileSize['*'] = (int)$rules;
+        } else {
+            $this->allowedMaxFileSize['*'] = $this->parseAndVerifyAllowedSize($rules);
         }
 
         return $this;
     }
 
+    /**
+     * Converts values in bytes or INI format to bytes.
+     * Will fall back to the default PHP size if null|0|''
+     * Verify the parsed size will not exceed PHP's ini settings
+     */
+    private function parseAndVerifyAllowedSize($value): int
+    {
+        $maxPHPAllows = $this->maxUploadSizeFromPHPIni();
+
+        if (is_numeric($value)) {
+            $tmpSize = (int)$value;
+        } elseif (is_string($value)) {
+            $tmpSize = Convert::memstring2bytes($value);
+        }
+
+        if (empty($tmpSize)) {
+            $tmpSize = $maxPHPAllows;
+        }
+
+        return min($maxPHPAllows, $tmpSize);
+    }
+
+    /**
+     * Checks the two ini settings to work out the max upload size php would allow
+     */
+    private function maxUploadSizeFromPHPIni(): int
+    {
+        $maxUpload = Convert::memstring2bytes(ini_get('upload_max_filesize'));
+        $maxPost = Convert::memstring2bytes(ini_get('post_max_size'));
+        return min($maxUpload, $maxPost);
+    }
+
     /**
      * @return array
      */
diff --git a/tests/php/UploadTest.php b/tests/php/UploadTest.php
index 74a41c24..71c8b695 100644
--- a/tests/php/UploadTest.php
+++ b/tests/php/UploadTest.php
@@ -2,6 +2,8 @@
 
 namespace SilverStripe\Assets\Tests;
 
+use ReflectionMethod;
+use SilverStripe\Core\Convert;
 use Silverstripe\Assets\Dev\TestAssetStore;
 use SilverStripe\Assets\File;
 use SilverStripe\Assets\Folder;
@@ -181,6 +183,65 @@ public function testAllowedFilesize()
         $this->assertFalse($result, 'Load failed because size was too big');
     }
 
+    public function testAllowedSizeWontExceedPHPConfig()
+    {
+        $v = new Upload_Validator();
+        $reflectionMethod = new ReflectionMethod($v, 'maxUploadSizeFromPHPIni');
+        $reflectionMethod->setAccessible(true);
+        $maxUploadSize = $reflectionMethod->invoke($v);
+
+        // Bump up the size by about 2MB.
+        // This would result in a hard PHP error if upload was attempted.
+        $exceedsMaxPHP = $maxUploadSize + 2000000;
+
+        $v->setAllowedMaxFileSize([
+            'jpg' => Convert::bytes2memstring($exceedsMaxPHP),
+            '*' => Convert::bytes2memstring($exceedsMaxPHP),
+        ]);
+
+        // Our config exceeded the ini settings for PHP, so fall back to that size
+        $this->assertEquals($maxUploadSize, $v->getAllowedMaxFileSize('jpg'));
+        $this->assertEquals($maxUploadSize, $v->getAllowedMaxFileSize('png'));
+    }
+
+    public function testWeFallBackToMaxPHPConfigIfEmptyArrayPassedForConfig()
+    {
+        $v = new Upload_Validator();
+        $reflectionMethod = new ReflectionMethod($v, 'maxUploadSizeFromPHPIni');
+        $reflectionMethod->setAccessible(true);
+        $maxUploadSize = $reflectionMethod->invoke($v);
+
+        $v->setAllowedMaxFileSize([]);
+
+        // We just fall back to default, which is the max php upload allowed
+        $this->assertEquals($maxUploadSize, $v->getAllowedMaxFileSize('jpg'));
+        $this->assertEquals($maxUploadSize, $v->getAllowedMaxFileSize('png'));
+    }
+
+    public function testWeHandleEmptyOrNullFileSizeValues()
+    {
+        $v = new Upload_Validator();
+        $reflectionMethod = new ReflectionMethod($v, 'maxUploadSizeFromPHPIni');
+        $reflectionMethod->setAccessible(true);
+        $maxUploadSize = $reflectionMethod->invoke($v);
+
+        /*
+         The test for the zip extension should be enabled once
+         the Convert::memstring2bytes if fixed to handle empty strings.
+         */
+        $v->setAllowedMaxFileSize([
+            'jpg' => '50',
+            'png' => 0,
+            'txt' => null,
+            // 'zip' => '',
+        ]);
+
+        $this->assertEquals(50, $v->getAllowedMaxFileSize('jpg'));
+        $this->assertEquals($maxUploadSize, $v->getAllowedMaxFileSize('png'));
+        $this->assertEquals($maxUploadSize, $v->getAllowedMaxFileSize('txt'));
+        // $this->assertEquals($maxUploadSize, $v->getAllowedMaxFileSize('zip'));
+    }
+
     public function testPHPUploadErrors()
     {
         $configMaxFileSizes = ['*' => '1k'];