LP: Security: sanitize params when executing converter

chamilo · Sep 5, 2023 · ed72914 · ed72914
1 parent 6f32625
commit ed72914
Show file tree

Hide file tree

Showing 4 changed files with 28 additions and 9 deletions.
diff --git a/main/lp/openoffice_document.class.php b/main/lp/openoffice_document.class.php
@@ -70,8 +70,8 @@ public function convert_document($file, $action_after_conversion = 'make_lp', $s
         if (!empty($size)) {
             list($w, $h) = explode('x', $size);
             if (!empty($w) && !empty($h)) {
-                $this->slide_width = $w;
-                $this->slide_height = $h;
+                $this->slide_width = (int) $w;
+                $this->slide_height = (int) $h;
             }
         }
 
@@ -106,6 +106,7 @@ public function convert_document($file, $action_after_conversion = 'make_lp', $s
 
             $files = [];
             $return = 0;
+            $cmd = escapeshellcmd($cmd);
             $shell = exec($cmd, $files, $return);
 
             if ($return != 0) { // If the java application returns an error code.
@@ -211,7 +212,9 @@ public function convertCopyDocument($originalPath, $convertedPath, $convertedTit
 
             $cmd .= ' -p '.api_get_setting('service_ppt2lp', 'port');
             // Call to the function implemented by child.
-            $cmd .= ' "'.$this->base_work_dir.'/'.$this->file_path.'"  "'.$this->base_work_dir.'/'.$this->created_dir.'"';
+            $cmd .= ' "'.Security::sanitizeExecParam($this->base_work_dir.'/'.$this->file_path)
+                .'"  "'
+                .Security::sanitizeExecParam($this->base_work_dir.'/'.$this->created_dir).'"';
             // To allow openoffice to manipulate docs.
             @chmod($this->base_work_dir, $permissionFolder);
             @chmod($this->base_work_dir.'/'.$this->file_path, $permissionFile);
@@ -221,6 +224,7 @@ public function convertCopyDocument($originalPath, $convertedPath, $convertedTit
 
             $files = [];
             $return = 0;
+            $cmd = escapeshellcmd($cmd);
             $shell = exec($cmd, $files, $return);
             // TODO: Chown is not working, root keep user privileges, should be www-data
             @chown($this->base_work_dir.'/'.$this->created_dir, 'www-data');

diff --git a/main/lp/openoffice_presentation.class.php b/main/lp/openoffice_presentation.class.php
@@ -247,16 +247,23 @@ public function make_lp($files = [])
     public function add_command_parameters()
     {
         if (empty($this->slide_width) || empty($this->slide_height)) {
-            list($this->slide_width, $this->slide_height) = explode('x', api_get_setting('service_ppt2lp', 'size'));
+            list($w, $h) = explode('x', api_get_setting('service_ppt2lp', 'size'));
+
+            $this->slide_width = (int) $w;
+            $this->slide_height = (int) $h;
         }
 
-        return ' -w '.$this->slide_width.' -h '.$this->slide_height.' -d oogie "'.$this->base_work_dir.'/'.$this->file_path.'"  "'.$this->base_work_dir.$this->created_dir.'.html"';
+        return ' -w '.$this->slide_width.' -h '.$this->slide_height.' -d oogie "'
+            .Security::sanitizeExecParam($this->base_work_dir.'/'.$this->file_path)
+            .'"  "'
+            .Security::sanitizeExecParam($this->base_work_dir.$this->created_dir.'.html')
+            .'"';
     }
 
     public function set_slide_size($width, $height)
     {
-        $this->slide_width = $width;
-        $this->slide_height = $height;
+        $this->slide_width = (int) $width;
+        $this->slide_height = (int) $height;
     }
 
     public function add_docs_to_visio($files = [])

diff --git a/main/lp/openoffice_text.class.php b/main/lp/openoffice_text.class.php
@@ -331,7 +331,11 @@ public function dealPerPage($header, $body)
      */
     public function add_command_parameters()
     {
-        return ' -d woogie "'.$this->base_work_dir.'/'.$this->file_path.'"  "'.$this->base_work_dir.$this->created_dir.'/'.$this->file_name.'.html"';
+        return ' -d woogie "'
+            .Security::sanitizeExecParam($this->base_work_dir.'/'.$this->file_path)
+            .'"  "'
+            .Security::sanitizeExecParam($this->base_work_dir.$this->created_dir.'/'.$this->file_name.'.html')
+            .'"';
     }
 
     /**

diff --git a/main/lp/openoffice_text_document.class.php b/main/lp/openoffice_text_document.class.php
@@ -333,7 +333,11 @@ public function dealPerPage($header, $body)
      */
     public function add_command_parameters()
     {
-        return ' -d woogie "'.$this->base_work_dir.'/'.$this->file_path.'"  "'.$this->base_work_dir.$this->created_dir.'/'.$this->file_name.'.html"';
+        return ' -d woogie "'
+            .Security::sanitizeExecParam($this->base_work_dir.'/'.$this->file_path)
+            .'"  "'
+            .Security::sanitizeExecParam($this->base_work_dir.$this->created_dir.'/'.$this->file_name.'.html')
+            .'"';
     }
 
     /**