Fixed an RCE vulnerability

brandonkelly · brandonkelly · commit 7359d18d4638 · 2023-06-27T16:35:52.000-07:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -6,8 +6,10 @@
 - The `up`, `migrate/up`, and `migrate/all` commands now revert any project config changes created by migrations on failure.
 - The `up`, `migrate/up`, and `migrate/all` commands now prompt to restore the backup created at the outset of the command, or recommend restoring a backup, on failure.
 - Added `craft\console\controllers\BackupTrait::restore()`.
+- Added `craft\helpers\Component::cleanseConfig()`.
 - Fixed a bug where Single entries weren’t getting preloaded for template macros, if the template body wasn‘t rendered. ([#13312](https://github.com/craftcms/cms/issues/13312))
 - Fixed a bug where asset folders could get dynamically created for elements with temporary slugs. ([#13311](https://github.com/craftcms/cms/issues/13311))
+- Fixed an RCE vulnerability.
 
 ## 4.4.14 - 2023-06-13
 
diff --git a/src/controllers/ConditionsController.php b/src/controllers/ConditionsController.php
@@ -11,6 +11,7 @@
 use craft\base\conditions\ConditionInterface;
 use craft\base\conditions\ConditionRuleInterface;
 use craft\helpers\ArrayHelper;
+use craft\helpers\Component;
 use craft\helpers\Json;
 use craft\web\Controller;
 use Illuminate\Support\Collection;
@@ -41,7 +42,7 @@ public function beforeAction($action): bool
         $this->requireCpRequest();
 
         $baseConfig = Json::decodeIfJson($this->request->getBodyParam('config'));
-        $config = $this->request->getBodyParam($baseConfig['name']);
+        $config = Component::cleanseConfig($this->request->getBodyParam($baseConfig['name']));
         $newRuleType = ArrayHelper::remove($config, 'new-rule-type');
         $conditionsService = Craft::$app->getConditions();
         $this->_condition = $conditionsService->createCondition($config);
diff --git a/src/helpers/Component.php b/src/helpers/Component.php
@@ -81,6 +81,27 @@ public static function validateComponentClass(string $class, ?string $instanceOf
         return true;
     }
 
+    /**
+     * Cleanses a component config of any `on X` or `as X` keys.
+     *
+     * @param array $config
+     * @return array
+     * @since 4.4.15
+     */
+    public static function cleanseConfig(array $config): array
+    {
+        foreach ($config as $key => $value) {
+            if (is_string($key) && (str_starts_with($key, 'on ') || str_starts_with($key, 'as '))) {
+                unset($config[$key]);
+                continue;
+            }
+            if (is_array($value)) {
+                $config[$key] = static::cleanseConfig($value);
+            }
+        }
+        return $config;
+    }
+
     /**
      * Instantiates and populates a component, and ensures that it is an instance of a given interface.
      *
diff --git a/tests/unit/helpers/ComponentHelperTest.php b/tests/unit/helpers/ComponentHelperTest.php
@@ -115,6 +115,16 @@ public function testIconSvg(string $needle, ?string $icon, string $label): void
         self::assertStringContainsString($needle, Component::iconSvg($icon, $label));
     }
 
+    /**
+     * @dataProvider cleanseConfigDataProvider
+     * @param array $expected
+     * @param array $config
+     */
+    public function testCleanseConfig(array $expected, array $config)
+    {
+        self::assertSame($expected, Component::cleanseConfig($config));
+    }
+
     /**
      * @return array
      */
@@ -304,4 +314,21 @@ public function iconSvgDataProvider(): array
             'aria-hidden' => ['aria-hidden="true"', '<svg width="100px" height="100px" viewBox="0 0 100 100" version="1.1" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink"></svg>', 'Default'],
         ];
     }
+
+    /**
+     * @return array
+     */
+    public function cleanseConfigDataProvider(): array
+    {
+        return [
+            [
+                ['f' => 'foo', 'b' => 'bar'],
+                ['f' => 'foo', 'b' => 'bar', 'as f' => 'f', 'on b' => 'b'],
+            ],
+            [
+                ['nested' => ['f' => 'foo', 'b' => 'bar']],
+                ['nested' => ['f' => 'foo', 'b' => 'bar', 'as f' => 'f', 'on b' => 'b']],
+            ],
+        ];
+    }
 }
