CVE-2020-26124: Improper Control of Generation of Code ('Code Injection') (CWE-94)
value: openmediavault before 4.1.36 and 5.x before 5.5.12 allows authenticated PHP code injection attacks, via the sortfield POST parameter of rpc.php, because json_encode_safe is not used in config/databasebackend.inc. Successful exploitation allows arbitrary command execution on the underlying operating system as root.
CWE description: The product constructs all or part of a code segment using externally-influenced input from an upstream component, but it does not neutralize or incorrectly neutralizes special elements that could modify the syntax or behavior of the intended code segment.
CVSS v3 base score: 8.8
Published: 2020-10-02T09:15Z
Repository: https://github.com/openmediavault/openmediavault
Language: PHP
Files changed: deb/openmediavault/usr/share/openmediavault/unittests/php/test_openmediavault_config_databasebackend.php, deb/openmediavault/usr/share/openmediavault/unittests/php/test_openmediavault_functions.php, deb/openmediavault/usr/share/php/openmediavault/config/databasebackend.inc, deb/openmediavault/usr/share/php/openmediavault/functions.inc
Commit date: 2020-09-26 22:36:04 +0200
Commit message: Cleanup code, add unit tests.

Signed-off-by: Volker Theile <votdev@gmx.de>

Diff: diff --git a/deb/openmediavault/usr/share/openmediavault/unittests/php/test_openmediavault_config_databasebackend.php b/deb/openmediavault/usr/share/openmediavault/unittests/php/test_openmediavault_config_databasebackend.php
new file mode 100644
index 000000000..d13d7740e
--- /dev/null
+++ b/deb/openmediavault/usr/share/openmediavault/unittests/php/test_openmediavault_config_databasebackend.php
@@ -0,0 +1,58 @@
+#!/usr/bin/phpunit -c/etc/openmediavault
+<?php
+/**
+ * This file is part of OpenMediaVault.
+ *
+ * @license   http://www.gnu.org/licenses/gpl.html GPL Version 3
+ * @author    Volker Theile <volker.theile@openmediavault.org>
+ * @copyright Copyright (c) 2009-2020 Volker Theile
+ *
+ * OpenMediaVault is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * any later version.
+ *
+ * OpenMediaVault is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with OpenMediaVault. If not, see <http://www.gnu.org/licenses/>.
+ */
+require_once("openmediavault/autoloader.inc");
+
+class test_openmediavault_config_databasebackend extends \PHPUnit\Framework\TestCase {
+	private $databaseBackend;
+
+	public function setUp() {
+		$this->databaseBackend = new \OMV\Config\DatabaseBackend(
+			sprintf("%s/../data/config.xml", getcwd()),
+			FALSE);
+		$this->databaseBackend->load();
+	}
+
+	public function test_compare_1() {
+		$this->assertEquals($this->databaseBackend->compare(
+			"//system/time",
+			[
+				"timezone" => "Europe/Berlin",
+				"ntp" => [
+					"enable" => 0,
+					"timeservers" => "pool.ntp.org",
+					"clients" => ""
+				]
+			]
+		), 0);
+	}
+
+	public function test_compare_2() {
+		$this->assertEquals($this->databaseBackend->compare(
+			"//system/powermanagement",
+			[
+				"cpufreq" => 0,
+				"powerbtn" => "nothing"
+			]
+		), -1);
+	}
+}
diff --git a/deb/openmediavault/usr/share/openmediavault/unittests/php/test_openmediavault_functions.php b/deb/openmediavault/usr/share/openmediavault/unittests/php/test_openmediavault_functions.php
index 07147a948..8abf3a2e6 100644
--- a/deb/openmediavault/usr/share/openmediavault/unittests/php/test_openmediavault_functions.php
+++ b/deb/openmediavault/usr/share/openmediavault/unittests/php/test_openmediavault_functions.php
@@ -333,6 +333,34 @@ class test_openmediavault_functions extends \PHPUnit\Framework\TestCase {
 	}
 
 	public function test_array_boolval_13() {
 		$this->assertTrue(array_boolval("a", "c", TRUE));
 	}
+
+	public function test_array_sort_key_1() {
+		$d = [
+			['id' => 3, 'text' => 'b'],
+			['id' => 1, 'text' => 'a'],
+			['id' => 2, 'text' => 'c']
+		];
+		$this->assertTrue(array_sort_key($d, "id"));
+		$this->assertEquals($d, [
+			['id' => 1, 'text' => 'a'],
+			['id' => 2, 'text' => 'c'],
+			['id' => 3, 'text' => 'b']
+		]);
+	}
+
+	public function test_array_sort_key_2() {
+		$d = [
+			['id' => 3, 'text' => 'b'],
+			['id' => 1, 'text' => 'a'],
+			['id' => 2, 'text' => 'c']
+		];
+		$this->assertTrue(array_sort_key($d, "text"));
+		$this->assertEquals($d, [
+			['id' => 1, 'text' => 'a'],
+			['id' => 3, 'text' => 'b'],
+			['id' => 2, 'text' => 'c']
+		]);
+	}
 }
diff --git a/deb/openmediavault/usr/share/php/openmediavault/config/databasebackend.inc b/deb/openmediavault/usr/share/php/openmediavault/config/databasebackend.inc
index 409c39c7e..77d3142ab 100644
--- a/deb/openmediavault/usr/share/php/openmediavault/config/databasebackend.inc
+++ b/deb/openmediavault/usr/share/php/openmediavault/config/databasebackend.inc
@@ -329,19 +329,34 @@ class DatabaseBackend {
 		if (!is_array($data))
 			$result = array($data);
 		// Convert all values to strings before comparison. This is necessary
 		// because the values returned from the configuration database are
 		// strings.
+		if (FALSE === array_walk_recursive($data, function(&$item, $key) {
+			if (is_string($item)) {
+				if (!mb_check_encoding($item, "UTF-8")) {
+					$item = utf8_encode($item);
+				}
+			} else {
+				$item = utf8_encode(strval($item));
+			}
+		})) {
+			return FALSE;
+		}
 		if (FALSE === array_walk_recursive($data,
 		  create_function('&$item, $key', 'if (is_string($item)) { '.
 		  'if (!mb_check_encoding($item, "UTF-8")) { '.
 		  '$item = utf8_encode($item); } } else { '.
 		  '$item = utf8_encode(strval($item)); }')))
 			return FALSE;
 		// Compare the arrays.
-		if (0 == count(array_diff($result, $data)))
+		// Note, we can not use `array_diff` here because this function
+		// does not support multidimensional arrays.
+		if (0 === strcmp(json_encode_safe($result),
+				json_encode_safe($data))) {
 			return 0;
+		}
 		return -1;
 	}
 
 	/**
 	 * Revert changes. All existing revision files will be deleted.
diff --git a/deb/openmediavault/usr/share/php/openmediavault/functions.inc b/deb/openmediavault/usr/share/php/openmediavault/functions.inc
index 83407eec3..710fe4a27 100644
--- a/deb/openmediavault/usr/share/php/openmediavault/functions.inc
+++ b/deb/openmediavault/usr/share/php/openmediavault/functions.inc
@@ -82,13 +82,15 @@ function array_expand(array $array, $separator = ".") {
  */
 function array_sort_key(array &$array, $key) {
 	if (!is_multi_array($array))
 		return FALSE;
 	// Sort the array.
-	if (FALSE === uasort($array, create_function('$a, $b',
-		"return strnatcmp(strval(\$a['$key']), strval(\$b['$key']));")))
+	if (FALSE === uasort($array, function($a, $b) use($key) {
+		return strnatcmp(strval($a[$key]), strval($b[$key]));
+	})) {
 		return FALSE;
+	}
 	// Re-index the array.
 	$array = array_values($array);
 	return TRUE;
 }
 

Vulnerable code: 		if (0 == count(array_diff($result, $data)))
	if (FALSE === uasort($array, create_function('$a, $b',
		"return strnatcmp(strval(\$a['$key']), strval(\$b['$key']));")))
Fixed code: #!/usr/bin/phpunit -c/etc/openmediavault
<?php
/**
 * This file is part of OpenMediaVault.
 *
 * @license   http://www.gnu.org/licenses/gpl.html GPL Version 3
 * @author    Volker Theile <volker.theile@openmediavault.org>
 * @copyright Copyright (c) 2009-2020 Volker Theile
 *
 * OpenMediaVault is free software: you can redistribute it and/or modify
 * it under the terms of the GNU General Public License as published by
 * the Free Software Foundation, either version 3 of the License, or
 * any later version.
 *
 * OpenMediaVault is distributed in the hope that it will be useful,
 * but WITHOUT ANY WARRANTY; without even the implied warranty of
 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 * GNU General Public License for more details.
 *
 * You should have received a copy of the GNU General Public License
 * along with OpenMediaVault. If not, see <http://www.gnu.org/licenses/>.
 */
require_once("openmediavault/autoloader.inc");

class test_openmediavault_config_databasebackend extends \PHPUnit\Framework\TestCase {
	private $databaseBackend;

	public function setUp() {
		$this->databaseBackend = new \OMV\Config\DatabaseBackend(
			sprintf("%s/../data/config.xml", getcwd()),
			FALSE);
		$this->databaseBackend->load();
	}

	public function test_compare_1() {
		$this->assertEquals($this->databaseBackend->compare(
			"//system/time",
			[
				"timezone" => "Europe/Berlin",
				"ntp" => [
					"enable" => 0,
					"timeservers" => "pool.ntp.org",
					"clients" => ""
				]
			]
		), 0);
	}

	public function test_compare_2() {
		$this->assertEquals($this->databaseBackend->compare(
			"//system/powermanagement",
			[
				"cpufreq" => 0,
				"powerbtn" => "nothing"
			]
		), -1);
	}
}

	public function test_array_sort_key_1() {
		$d = [
			['id' => 3, 'text' => 'b'],
			['id' => 1, 'text' => 'a'],
			['id' => 2, 'text' => 'c']
		];
		$this->assertTrue(array_sort_key($d, "id"));
		$this->assertEquals($d, [
			['id' => 1, 'text' => 'a'],
			['id' => 2, 'text' => 'c'],
			['id' => 3, 'text' => 'b']
		]);
	}

	public function test_array_sort_key_2() {
		$d = [
			['id' => 3, 'text' => 'b'],
			['id' => 1, 'text' => 'a'],
			['id' => 2, 'text' => 'c']
		];
		$this->assertTrue(array_sort_key($d, "text"));
		$this->assertEquals($d, [
			['id' => 1, 'text' => 'a'],
			['id' => 3, 'text' => 'b'],
			['id' => 2, 'text' => 'c']
		]);
	}
		if (FALSE === array_walk_recursive($data, function(&$item, $key) {
			if (is_string($item)) {
				if (!mb_check_encoding($item, "UTF-8")) {
					$item = utf8_encode($item);
				}
			} else {
				$item = utf8_encode(strval($item));
			}
		})) {
			return FALSE;
		}
		// Note, we can not use `array_diff` here because this function
		// does not support multidimensional arrays.
		if (0 === strcmp(json_encode_safe($result),
				json_encode_safe($data))) {
		}
	if (FALSE === uasort($array, function($a, $b) use($key) {
		return strnatcmp(strval($a[$key]), strval($b[$key]));
	})) {
	}