From 6442976c25930cb370c65a22784b9caee7ed1de2 Mon Sep 17 00:00:00 2001
From: Seb Dangerfield <1449113+sedan07@users.noreply.github.com>
Date: Fri, 27 Aug 2021 15:12:18 +0100
Subject: [PATCH] Prevent new lines and variables in settings values

GHSA-88f9-7xxh-c688 and GHSA-9jxw-cfrh-jxq6
---
 .../Setting/InvalidSettingValueException.php  | 22 +++++
 .../Setting/SettingExceptionInterface.php     | 22 +++++
 .../Config/UpdateConfigCommandHandler.php     | 92 +++++++++++++++----
 3 files changed, 118 insertions(+), 18 deletions(-)
 create mode 100644 app/Bus/Exceptions/Setting/InvalidSettingValueException.php
 create mode 100644 app/Bus/Exceptions/Setting/SettingExceptionInterface.php

diff --git a/app/Bus/Exceptions/Setting/InvalidSettingValueException.php b/app/Bus/Exceptions/Setting/InvalidSettingValueException.php
new file mode 100644
index 000000000000..bf7004c63ee9
--- /dev/null
+++ b/app/Bus/Exceptions/Setting/InvalidSettingValueException.php
@@ -0,0 +1,22 @@
+<?php
+
+/*
+ * This file is part of Cachet.
+ *
+ * (c) FiveAI Ltd
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace CachetHQ\Cachet\Bus\Exceptions\Setting;
+
+use Exception;
+
+/**
+ * This is the invalid setting value exception.
+ */
+class InvalidSettingValueException extends Exception implements SettingExceptionInterface
+{
+    //
+}
diff --git a/app/Bus/Exceptions/Setting/SettingExceptionInterface.php b/app/Bus/Exceptions/Setting/SettingExceptionInterface.php
new file mode 100644
index 000000000000..61b5de74d1ac
--- /dev/null
+++ b/app/Bus/Exceptions/Setting/SettingExceptionInterface.php
@@ -0,0 +1,22 @@
+<?php
+
+/*
+ * This file is part of Cachet.
+ *
+ * (c) FiveAI Ltd
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace CachetHQ\Cachet\Bus\Exceptions\Setting;
+
+use CachetHQ\Cachet\Bus\Exceptions\ExceptionInterface;
+
+/**
+ * This is the setting exception interface.
+ */
+interface SettingExceptionInterface extends ExceptionInterface
+{
+    //
+}
diff --git a/app/Bus/Handlers/Commands/System/Config/UpdateConfigCommandHandler.php b/app/Bus/Handlers/Commands/System/Config/UpdateConfigCommandHandler.php
index ff7d1f4e101e..083ad2866845 100644
--- a/app/Bus/Handlers/Commands/System/Config/UpdateConfigCommandHandler.php
+++ b/app/Bus/Handlers/Commands/System/Config/UpdateConfigCommandHandler.php
@@ -12,8 +12,9 @@
 namespace CachetHQ\Cachet\Bus\Handlers\Commands\System\Config;
 
 use CachetHQ\Cachet\Bus\Commands\System\Config\UpdateConfigCommand;
+use CachetHQ\Cachet\Bus\Exceptions\Setting\InvalidSettingValueException;
 use Dotenv\Dotenv;
-use Dotenv\Exception\InvalidPathException;
+use Illuminate\Filesystem\Filesystem;
 
 /**
  * This is the update config command handler class.
@@ -22,6 +23,29 @@
  */
 class UpdateConfigCommandHandler
 {
+    /**
+     * @var string
+     */
+    protected $dir;
+
+    /**
+     * @var string
+     */
+    protected $file;
+
+    /**
+     * @var string
+     */
+    protected $path;
+
+    public function __construct(Filesystem $filesystem)
+    {
+        $this->dir = app()->environmentPath();
+        $this->file = app()->environmentFile();
+        $this->path = "{$this->dir}/{$this->file}";
+        $this->filesystem = $filesystem;
+    }
+
     /**
      * Handle update config command handler instance.
      *
@@ -34,6 +58,38 @@ public function handle(UpdateConfigCommand $command)
         foreach ($command->values as $setting => $value) {
             $this->writeEnv($setting, $value);
         }
+
+        if (app()->configurationIsCached()) {
+            $this->filesystem->delete(app()->getCachedConfigPath());
+        }
+    }
+
+    /**
+     * Replicate phpdotenv's nested variable resolution to identify if a given
+     * value contains such sequences.
+     *
+     * @param string $value
+     *
+     * @return string
+     */
+    protected function resolveNestedVariables($value)
+    {
+        if (strpos($value, '$') !== false) {
+            $value = preg_replace_callback(
+                '/\${([a-zA-Z0-9_.]+)}/',
+                function ($matchedPatterns) {
+                    $nestedVariable = env($matchedPatterns[1]);
+                    if ($nestedVariable === null) {
+                        return $matchedPatterns[0];
+                    } else {
+                        return $nestedVariable;
+                    }
+                },
+                $value
+            );
+        }
+
+        return $value;
     }
 
     /**
@@ -46,23 +102,23 @@ public function handle(UpdateConfigCommand $command)
      */
     protected function writeEnv($key, $value)
     {
-        $dir = app()->environmentPath();
-        $file = app()->environmentFile();
-        $path = "{$dir}/{$file}";
-
-        try {
-            (new Dotenv($dir, $file))->load();
-
-            $envKey = strtoupper($key);
-            $envValue = env($envKey) ?: 'null';
-
-            file_put_contents($path, str_replace(
-                "{$envKey}={$envValue}",
-                "{$envKey}={$value}",
-                file_get_contents($path)
-            ));
-        } catch (InvalidPathException $e) {
-            throw $e;
+        if (strstr($key, "\n") !== false || strstr($value, "\n") !== false) {
+            throw new InvalidSettingValueException('New setting key or value contains new lines');
         }
+
+        (new Dotenv($this->dir, $this->file))->safeLoad();
+
+        $envKey = strtoupper($key);
+        $envValue = env($envKey) ?: 'null';
+
+        if ($this->resolveNestedVariables($value) !== $value) {
+            throw new InvalidSettingValueException("New setting key $envKey contains a nested variable");
+        }
+
+        file_put_contents($this->path, str_replace(
+            "{$envKey}={$envValue}",
+            "{$envKey}={$value}",
+            file_get_contents($this->path)
+        ));
     }
 }