Security: Better fix for CVE-2020-12641

CHANGELOG

@@ -3,6 +3,7 @@ CHANGELOG Roundcube Webmail
 
 RELEASE 1.3.12
 --------------
+ - Security: Better fix for CVE-2020-12641
  - Security: Fix XSS issue in template object 'username' (#7406) 
  - Security: Fix couple of XSS issues in Installer (#7406)
 

program/lib/Roundcube/rcube_image.php

@@ -100,7 +100,7 @@ class rcube_image
     {
         $result  = false;
         $rcube   = rcube::get_instance();
-        $convert = $rcube->config->get('im_convert_path', false);
+        $convert = self::getCommand('im_convert_path');
         $props   = $this->props();
 
         if (empty($props)) {
@@ -159,7 +159,7 @@ class rcube_image
                             'size'    => $width . 'x' . $height,
                         );
 
-                        $result = rcube::exec(escapeshellcmd($convert)
+                        $result = rcube::exec($convert
                             . ' 2>&1 -flatten -auto-orient -colorspace sRGB -strip'
                             . ' -quality {quality} -resize {size} {intype}:{in} {type}:{out}', $p);
                     }
@@ -308,7 +308,7 @@ class rcube_image
     public function convert($type, $filename = null)
     {
         $rcube   = rcube::get_instance();
-        $convert = $rcube->config->get('im_convert_path', false);
+        $convert = self::getCommand('im_convert_path');
 
         if (!$filename) {
             $filename = $this->image_file;
@@ -325,8 +325,7 @@ class rcube_image
             $p['out']  = $filename;
             $p['type'] = self::$extensions[$type];
 
-            $result = rcube::exec(escapeshellcmd($convert)
+            $result = rcube::exec($convert . ' 2>&1 -colorspace sRGB -strip -flatten -quality 75 {in} {type}:{out}', $p);
-                . ' 2>&1 -colorspace sRGB -strip -flatten -quality 75 {in} {type}:{out}', $p);
 
             if ($result === '') {
                 chmod($filename, 0600);
@@ -407,7 +406,7 @@ class rcube_image
         $rcube = rcube::get_instance();
 
         // @TODO: check if specified mimetype is really supported
-        return class_exists('Imagick', false) || $rcube->config->get('im_convert_path');
+        return class_exists('Imagick', false) || self::getCommand('im_convert_path');
     }
 
     /**
@@ -418,9 +417,9 @@ class rcube_image
         $rcube = rcube::get_instance();
 
         // use ImageMagick in command line
-        if ($cmd = $rcube->config->get('im_identify_path')) {
+        if ($cmd = self::getCommand('im_identify_path')) {
             $args = array('in' => $this->image_file, 'format' => "%m %[fx:w] %[fx:h]");
-            $id   = rcube::exec(escapeshellcmd($cmd) . ' 2>/dev/null -format {format} {in}', $args);
+            $id   = rcube::exec($cmd . ' 2>/dev/null -format {format} {in}', $args);
 
             if ($id) {
                 return explode(' ', strtolower($id));
@@ -459,4 +458,39 @@ class rcube_image
         $size = $props['width'] * $props['height'] * $multip;
         return rcube_utils::mem_check($size);
     }
+
+    /**
+     * Get the configured command and make sure it is safe to use.
+     * We cannot trust configuration, and escapeshellcmd() is useless.
+     *
+     * @param string $opt_name Configuration option name
+     *
+     * @return bool|string The command or False if not set or invalid
+     */
+    private static function getCommand($opt_name)
+    {
+        static $error = [];
+
+        $cmd = rcube::get_instance()->config->get($opt_name);
+
+        if (empty($cmd)) {
+            return false;
+        }
+
+        if (preg_match('/^(convert|identify)(\.exe)?$/i', $cmd)) {
+            return $cmd;
+        }
+
+        // Executable must exist, also disallow network shares on Windows
+        if ($cmd[0] != "\\" && file_exists($cmd)) {
+            return $cmd;
+        }
+
+        if (empty($error[$opt_name])) {
+            rcube::raise_error("Invalid $opt_name: $cmd", true, false);
+            $error[$opt_name] = true;
+        }
+
+        return false;
+    }
 }