aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorMario <mario@mariovavti.com>2023-12-20 10:27:56 +0000
committerMario <mario@mariovavti.com>2023-12-20 10:27:56 +0000
commitb15e521b0eebba7001dc87be0bc9fe0cee19aa23 (patch)
tree80ad23fd442ba5f7d7b3a6386b658b37569965aa
parent63c401e6d63e166ff8f545f06aa55503882871bb (diff)
parent9c184ddfd0e986af7bb99a45a3c7c8f1bf616035 (diff)
downloadvolse-hubzilla-b15e521b0eebba7001dc87be0bc9fe0cee19aa23.tar.gz
volse-hubzilla-b15e521b0eebba7001dc87be0bc9fe0cee19aa23.tar.bz2
volse-hubzilla-b15e521b0eebba7001dc87be0bc9fe0cee19aa23.zip
Merge branch 'fix-config-deserialization' into 'dev'
Fix deserialization of config values broken by 69266cd6. See merge request hubzilla/core!2077
-rw-r--r--Zotlabs/Lib/Config.php21
-rw-r--r--tests/unit/Lib/ConfigTest.php61
2 files changed, 76 insertions, 6 deletions
diff --git a/Zotlabs/Lib/Config.php b/Zotlabs/Lib/Config.php
index 3d34c8497..b1754df09 100644
--- a/Zotlabs/Lib/Config.php
+++ b/Zotlabs/Lib/Config.php
@@ -72,7 +72,7 @@ class Config {
*/
public static function Set($family, $key, $value) {
// manage array value
- $dbvalue = ((is_array($value)) ? serialise($value) : $value);
+ $dbvalue = ((is_array($value)) ? 'json:' . json_encode($value) : $value);
$dbvalue = ((is_bool($dbvalue)) ? intval($dbvalue) : $dbvalue);
if (self::Get($family, $key) === false || (! self::get_from_storage($family, $key))) {
@@ -136,11 +136,20 @@ class Config {
return $default;
}
- return ((! is_array(App::$config[$family][$key])) && (preg_match('|^a:[0-9]+:{.*}$|s', App::$config[$family][$key]))
- ? unserialize(App::$config[$family][$key])
- : App::$config[$family][$key]
- );
-
+ $value = App::$config[$family][$key];
+
+ if (! is_array($value)) {
+ if (substr($value, 0, 5) == 'json:') {
+ return json_decode(substr($value, 5), true);
+ } else if (preg_match('|^a:[0-9]+:{.*}$|s', $value)) {
+ // Unserialize in inherently unsafe. Try to mitigate by not
+ // allowing unserializing objects. Only kept for backwards
+ // compatibility. JSON serialization should be prefered.
+ return unserialize($value, array('allowed_classes' => false));
+ } else {
+ return $value;
+ }
+ }
}
return $default;
diff --git a/tests/unit/Lib/ConfigTest.php b/tests/unit/Lib/ConfigTest.php
new file mode 100644
index 000000000..a8ae3631b
--- /dev/null
+++ b/tests/unit/Lib/ConfigTest.php
@@ -0,0 +1,61 @@
+<?php
+declare(strict_types=1);
+
+/**
+ * Tests for the Zotlabs\Lib\Config class.
+ *
+ * Until we have database testing in place, we can only test the Congig::Get
+ * method for now. This should be improved once the database test framework is
+ * merged.
+ */
+class ConfigTest extends Zotlabs\Tests\Unit\UnitTestCase {
+ /*
+ * Hardcode a config that we can test against, and that we can
+ * reuse in all the test cases.
+ */
+ public function setUp(): void {
+ \App::$config = array(
+ 'test' => array (
+ 'plain' => 'plain value',
+ 'php-array' => 'a:3:{i:0;s:3:"one";i:1;s:3:"two";i:2;s:5:"three";}',
+ 'json-array' => 'json:["one","two","three"]',
+ 'object-injection' => 'a:1:{i:0;O:18:"Zotlabs\Lib\Config":0:{}}',
+ 'config_loaded' => true,
+ ),
+ );
+ }
+
+ public function testGetPlainTextValue(): void {
+ $this->assertEquals(
+ Zotlabs\Lib\Config::Get('test', 'plain'),
+ 'plain value'
+ );
+ }
+
+ public function testGetJSONSerializedArray(): void {
+ $this->assertEquals(
+ Zotlabs\Lib\Config::Get('test', 'json-array'),
+ array('one', 'two', 'three')
+ );
+ }
+
+ /*
+ * Test that we can retreive old style serialized arrays that were
+ * serialized with th PHP `serialize()` function.
+ */
+ public function testGetPHPSerializedArray(): void {
+ $this->assertEquals(
+ Zotlabs\Lib\Config::Get('test', 'php-array'),
+ array('one', 'two', 'three')
+ );
+ }
+
+ /*
+ * Make sure we're not vulnerable to PHP Object injection attacks when
+ * using the PHP `unserialize()` function.
+ */
+ public function testGetMaliciousPHPSerializedArray(): void {
+ $value = Zotlabs\Lib\Config::Get('test', 'object-injection');
+ $this->assertEquals($value[0]::class, '__PHP_Incomplete_Class');
+ }
+}