Merge commit from fork

* fix: prevent command injection in ImageMagick handler

* fix: prevent command injection in ImageMagick _text() method

* add code suggestion from the code review

Co-authored-by: John Paul E. Balandan, CPA <[email protected]>

* add code suggestion from the code review

Co-authored-by: John Paul E. Balandan, CPA <[email protected]>

---------

Co-authored-by: John Paul E. Balandan, CPA <[email protected]>

Author: michalsn

system/Images/Handlers/ImageMagickHandler.php

@@ -82,8 +82,8 @@ public function _resize(bool $maintainRatio = false)
         }
 
         $action = $maintainRatio
-            ? ' -resize ' . ($this->width ?? 0) . 'x' . ($this->height ?? 0) . ' "' . $source . '" "' . $destination . '"'
-            : ' -resize ' . ($this->width ?? 0) . 'x' . ($this->height ?? 0) . "{$escape}! \"" . $source . '" "' . $destination . '"';
+            ? ' -resize ' . ($this->width ?? 0) . 'x' . ($this->height ?? 0) . ' ' . escapeshellarg($source) . ' ' . escapeshellarg($destination)
+            : ' -resize ' . ($this->width ?? 0) . 'x' . ($this->height ?? 0) . "{$escape}! " . escapeshellarg($source) . ' ' . escapeshellarg($destination);
 
         $this->process($action);
 
@@ -354,7 +354,7 @@ protected function _text(string $text, array $options = [])
 
         // Font
         if (! empty($options['fontPath'])) {
-            $cmd .= " -font '{$options['fontPath']}'";
+            $cmd .= ' -font ' . escapeshellarg($options['fontPath']);
         }
 
         if (isset($options['hAlign'], $options['vAlign'])) {
@@ -393,28 +393,28 @@ protected function _text(string $text, array $options = [])
             $xAxis = $xAxis >= 0 ? '+' . $xAxis : $xAxis;
             $yAxis = $yAxis >= 0 ? '+' . $yAxis : $yAxis;
 
-            $cmd .= " -gravity {$gravity} -geometry {$xAxis}{$yAxis}";
+            $cmd .= ' -gravity ' . escapeshellarg($gravity) . ' -geometry ' . escapeshellarg("{$xAxis}{$yAxis}");
         }
 
         // Color
         if (isset($options['color'])) {
             [$r, $g, $b] = sscanf("#{$options['color']}", '#%02x%02x%02x');
 
-            $cmd .= " -fill 'rgba({$r},{$g},{$b},{$options['opacity']})'";
+            $cmd .= ' -fill ' . escapeshellarg("rgba({$r},{$g},{$b},{$options['opacity']})");
         }
 
         // Font Size - use points....
         if (isset($options['fontSize'])) {
-            $cmd .= " -pointsize {$options['fontSize']}";
+            $cmd .= ' -pointsize ' . escapeshellarg((string) $options['fontSize']);
         }
 
         // Text
-        $cmd .= " -annotate 0 '{$text}'";
+        $cmd .= ' -annotate 0 ' . escapeshellarg($text);
 
         $source      = ! empty($this->resource) ? $this->resource : $this->image()->getPathname();
         $destination = $this->getResourcePath();
 
-        $cmd = " '{$source}' {$cmd} '{$destination}'";
+        $cmd = ' ' . escapeshellarg($source) . ' ' . $cmd . ' ' . escapeshellarg($destination);
 
         $this->process($cmd);
     }

tests/system/Images/ImageMagickHandlerTest.php

@@ -487,4 +487,65 @@ public function testImageReorientPortrait(): void
             $this->assertSame(['red' => 62, 'green' => 62, 'blue' => 62, 'alpha' => 0], $rgb);
         }
     }
+
+    public function testCommandInjectionPrevention(): void
+    {
+        $injectionFile     = 'ci4_security_test.txt';
+        $maliciousFilename = 'image.png`echo 123456 | tee ' . $injectionFile . '`';
+        $tempPath          = $this->root . $maliciousFilename;
+
+        $source = $this->origin . 'rocket.png';
+
+        if (file_exists($injectionFile)) {
+            unlink($injectionFile);
+        }
+
+        file_put_contents($tempPath, file_get_contents($source));
+
+        try {
+            $this->handler->withFile($tempPath);
+            $this->handler->resize(50, 50);
+        } catch (ImageException) {
+            $this->fail('Command injection succeeded. Fix is incomplete.');
+        } finally {
+            // Check that the command injection file was NOT created
+            $this->assertFileDoesNotExist($injectionFile);
+
+            // Verify the image processing still works normally
+            $this->assertSame(50, $this->handler->getWidth());
+            $this->assertSame(50, $this->handler->getHeight());
+
+            if (file_exists($tempPath)) {
+                unlink($tempPath);
+            }
+        }
+    }
+
+    public function testCommandInjectionPreventionWithText(): void
+    {
+        $injectionFile = 'ci4_security_test.txt';
+        $tempFilename  = 'image.png';
+        $tempPath      = $this->root . $tempFilename;
+
+        $source = $this->origin . 'rocket.png';
+
+        if (file_exists($injectionFile)) {
+            unlink($injectionFile);
+        }
+
+        file_put_contents($tempPath, file_get_contents($source));
+
+        try {
+            $this->handler->withFile($tempPath);
+            $text = "Hello'; echo 123456 > {$injectionFile}; echo 'World";
+            $this->handler->text($text);
+        } finally {
+            // Check that the command injection file was NOT created
+            $this->assertFileDoesNotExist($injectionFile);
+
+            if (file_exists($tempPath)) {
+                unlink($tempPath);
+            }
+        }
+    }
 }

user_guide_src/source/changelogs/v4.6.2.rst

@@ -10,6 +10,14 @@ Release Date: Unreleased
     :local:
     :depth: 3
 
+********
+SECURITY
+********
+
+- **ImageMagick Handler:** *Command Injection Vulnerability in ImageMagick Handler* was fixed.
+  See the `Security advisory GHSA-9952-gv64-x94c <https://github.com/codeigniter4/CodeIgniter4/security/advisories/GHSA-9952-gv64-x94c>`_
+  for more information.
+
 ********
 BREAKING
 ********