Security: Sanitize language variable inputs and improve variable handling in multiple files

AngelFQC · AngelFQC · commit e1c7879d6317 · 2025-04-15T18:12:25.000-05:00
diff --git a/main/admin/sub_language_ajax.inc.php b/main/admin/sub_language_ajax.inc.php
@@ -14,10 +14,15 @@
 api_protect_admin_script();
 
 $new_language = Security::remove_XSS($_REQUEST['new_language']);
-$language_variable = Security::remove_XSS($_REQUEST['variable_language']);
+$language_variable = ltrim(
+    Security::remove_XSS($_REQUEST['variable_language']),
+    '$'
+);
 $file_id = intval($_REQUEST['file_id']);
 
-if (isset($new_language) && isset($language_variable) && isset($file_id)) {
+$variableIsValid = isset($language_variable) && preg_match('/^[a-zA-Z_][a-zA-Z0-9_]*$/', $language_variable);
+
+if (isset($new_language) && $variableIsValid && isset($file_id)) {
     $file_language = $language_files_to_load[$file_id].'.inc.php';
     $id_language = intval($_REQUEST['id']);
     $sub_language_id = intval($_REQUEST['sub']);
@@ -27,12 +32,7 @@
     $all_file_of_directory = SubLanguageManager::get_all_language_variable_in_file($path_folder);
     $return_value = SubLanguageManager::add_file_in_language_directory($path_folder);
 
-    //update variable language
-    // Replace double quotes to avoid parse errors
-    $new_language = str_replace('"', '\"', $new_language);
-    // Replace new line signs to avoid parse errors - see #6773
-    $new_language = str_replace("\n", "\\n", $new_language);
-    $all_file_of_directory[$language_variable] = "\"".$new_language."\";";
+    $all_file_of_directory[$language_variable] = $new_language;
     $result_array = [];
 
     foreach ($all_file_of_directory as $key_value => $value_info) {
diff --git a/main/cron/lang/langstats.class.php b/main/cron/lang/langstats.class.php
@@ -190,10 +190,7 @@ public function get_variables_origin()
         $vars = [];
         $priority = ['trad4all'];
         foreach ($priority as $file) {
-            $list = SubLanguageManager::get_all_language_variable_in_file(
-                $path.$file.'.inc.php',
-                true
-            );
+            $list = SubLanguageManager::get_all_language_variable_in_file($path.$file.'.inc.php');
             foreach ($list as $var => $trad) {
                 $vars[$var] = $file.'.inc.php';
             }
@@ -203,10 +200,7 @@ public function get_variables_origin()
             if (substr($file, 0, 1) == '.' or in_array($file, $priority)) {
                 continue;
             }
-            $list = SubLanguageManager::get_all_language_variable_in_file(
-                $path.$file,
-                true
-            );
+            $list = SubLanguageManager::get_all_language_variable_in_file($path.$file);
             foreach ($list as $var => $trad) {
                 $vars[$var] = $file;
             }
diff --git a/main/cron/lang/list_undefined_langvars.php b/main/cron/lang/list_undefined_langvars.php
@@ -20,7 +20,7 @@
 foreach ($list as $entry) {
     $file = $path.'/'.$entry;
     if (is_file($file)) {
-        $terms = array_merge($terms, SubLanguageManager::get_all_language_variable_in_file($file, true));
+        $terms = array_merge($terms, SubLanguageManager::get_all_language_variable_in_file($file));
     }
 }
 // get only the array keys (the language variables defined in language files)
diff --git a/main/cron/lang/list_unused_langvars.php b/main/cron/lang/list_unused_langvars.php
@@ -20,7 +20,7 @@
 foreach ($list as $entry) {
     $file = $path.'/'.$entry;
     if (is_file($file)) {
-        $terms = array_merge($terms, SubLanguageManager::get_all_language_variable_in_file($file, true));
+        $terms = array_merge($terms, SubLanguageManager::get_all_language_variable_in_file($file));
     }
 }
 // get only the array keys (the language variables defined in language files)
diff --git a/main/cron/lang/switch_files_to_gettext.php b/main/cron/lang/switch_files_to_gettext.php
@@ -20,7 +20,7 @@
 foreach ($list as $entry) {
     $file = $path.'/'.$entry;
     if (is_file($file)) {
-        $terms = array_merge($terms, SubLanguageManager::get_all_language_variable_in_file($file, true));
+        $terms = array_merge($terms, SubLanguageManager::get_all_language_variable_in_file($file));
     }
 }
 foreach ($terms as $index => $translation) {
diff --git a/main/inc/ajax/lang.ajax.php b/main/inc/ajax/lang.ajax.php
@@ -23,7 +23,10 @@
         Security::clear_token();
         if (isset($_REQUEST['new_language']) && isset($_REQUEST['variable_language']) && isset($_REQUEST['category_id'])) {
             $newLanguage = Security::remove_XSS($_REQUEST['new_language']);
-            $langVariable = Security::remove_XSS($_REQUEST['variable_language']);
+            $langVariable = ltrim(
+                Security::remove_XSS($_REQUEST['variable_language']),
+                '$'
+            );
             $categoryId = (int) $_REQUEST['category_id'];
             $languageId = (int) $_REQUEST['id'];
             $subLanguageId = (int) $_REQUEST['sub'];
@@ -41,10 +44,7 @@
             $returnValue = SubLanguageManager::add_file_in_language_directory($pathFolder);
 
             //update variable language
-            // Replace double quotes to avoid parse errors
-            $newLanguage = str_replace('"', '\"', $newLanguage);
-            $newLanguage = str_replace("\n", "\\n", $newLanguage);
-            $allFileOfDirectory[$langVariable] = "\"".$newLanguage."\";";
+            $allFileOfDirectory[$langVariable] = $newLanguage;
 
             $resultArray = [];
             foreach ($allFileOfDirectory as $key => $value) {
diff --git a/main/inc/lib/sub_language.class.php b/main/inc/lib/sub_language.class.php
@@ -119,31 +119,23 @@ public static function get_all_information_of_language($parent_id)
      * Get all information of chamilo file.
      *
      * @param string $system_path_file    The chamilo path file (/var/www/chamilo/main/lang/spanish/gradebook.inc.php)
-     * @param bool   $get_as_string_index Whether we want to remove the '$' prefix in the results or not
      *
      * @return array Contains all information of chamilo file
      */
-    public static function get_all_language_variable_in_file($system_path_file, $get_as_string_index = false)
-    {
-        $res_list = [];
-        if (!is_readable($system_path_file)) {
-            return $res_list;
-        }
-        $info_file = file($system_path_file);
-        foreach ($info_file as $line) {
-            if (substr($line, 0, 1) != '$') {
-                continue;
-            }
-            list($var, $val) = explode('=', $line, 2);
-            $var = trim($var);
-            $val = trim($val);
-            if ($get_as_string_index) { //remove the prefix $
-                $var = substr($var, 1);
-            }
-            $res_list[$var] = $val;
-        }
+    public static function get_all_language_variable_in_file(string $system_path_file): array {
+        ob_start();
+
+        include $system_path_file;
 
-        return $res_list;
+        ob_end_clean();
+
+        $variables = get_defined_vars();
+
+        unset($variables['system_path_file']);
+        unset($variables['get_as_string_index']);
+        unset($variables['php_errormsg']);
+
+        return $variables;
     }
 
     /**
@@ -171,16 +163,16 @@ public static function add_file_in_language_directory($system_path_file)
      */
     public static function write_data_in_file($path_file, $new_term, $new_variable)
     {
+        // Replace double quotes to avoid parse errors
+        $new_term = addcslashes($new_term, "\$\"\\");
+        // Replace new line signs to avoid parse errors - see #6773
+        $new_term = str_replace("\n", "\\n", $new_term);
+
         $return_value = false;
-        $new_data = $new_variable.'='.$new_term;
+        $new_data = '$'.$new_variable.'="'.$new_term.'";'.PHP_EOL;
         $resource = @fopen($path_file, "a");
         if (file_exists($path_file) && $resource) {
-            if (fwrite($resource, $new_data.PHP_EOL) === false) {
-                //not allow to write
-                $return_value = false;
-            } else {
-                $return_value = true;
-            }
+            $return_value = !(fwrite($resource, $new_data) === false);
             fclose($resource);
         }
 
diff --git a/tests/scripts/build_translation_request_file.php b/tests/scripts/build_translation_request_file.php
@@ -19,7 +19,7 @@
 $referenceTerms = array();
 $file = $path . $referenceLanguage . '/trad4all.inc.php';
 if (is_file($file)) {
-    $referenceTerms = array_merge($referenceTerms, SubLanguageManager::get_all_language_variable_in_file($file,true));
+    $referenceTerms = array_merge($referenceTerms, SubLanguageManager::get_all_language_variable_in_file($file));
 }
 // get only the array keys (the language variables defined in language files)
 $definedTerms = array_keys($referenceTerms);
@@ -32,7 +32,7 @@
 $l = strlen(api_get_path(SYS_PATH));
 $file = $path . $language . '/trad4all.inc.php';
 if (is_file($file)) {
-    $nonMissingTerms = array_merge($nonMissingTerms, SubLanguageManager::get_all_language_variable_in_file($file,true));
+    $nonMissingTerms = array_merge($nonMissingTerms, SubLanguageManager::get_all_language_variable_in_file($file));
 }
 $nonMissingTerms = array_keys($nonMissingTerms);
 //print_r($nonMissingTerms);

Original file line number	Diff line number	Diff line change
`@@ -20,7 +20,7 @@`
`20`	`20`	`foreach ($list as $entry) {`
`21`	`21`	`$file = $path.'/'.$entry;`
`22`	`22`	`if (is_file($file)) {`
`23`		`- $terms = array_merge($terms, SubLanguageManager::get_all_language_variable_in_file($file, true));`
	`23`	`+ $terms = array_merge($terms, SubLanguageManager::get_all_language_variable_in_file($file));`
`24`	`24`	`}`
`25`	`25`	`}`
`26`	`26`	`// get only the array keys (the language variables defined in language files)`