CVE-2023-52085: Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal') (CWE-22)
value: Winter is a free, open-source content management system. Users with access to backend forms that include a ColorPicker FormWidget can provide a value that would then be included without further processing in the compilation of custom stylesheets via LESS. This had the potential to lead to a Local File Inclusion vulnerability. This issue has been patched in v1.2.4.
CWE description: The product uses external input to construct a pathname that is intended to identify a file or directory that is located underneath a restricted parent directory, but the product does not properly neutralize special elements within the pathname that can cause the pathname to resolve to a location that is outside of the restricted directory.
CVSS v3 base score: 5.4
Published: 2023-12-29T00:15Z
Repository: https://github.com/wintercms/winter
Language: PHP
Files changed: modules/backend/formwidgets/ColorPicker.php, modules/backend/lang/en/lang.php, modules/backend/tests/formwidgets/ColorPickerTest.php
Commit date: 2023-12-14 00:18:03 -0600
Commit message: Added validation check for colourpicker form widget (#1020)

Co-authored-by: Luke Towers <luke@luketowers.ca>

Diff: diff --git a/modules/backend/formwidgets/ColorPicker.php b/modules/backend/formwidgets/ColorPicker.php
index 60fd4140f0..247271f798 100644
--- a/modules/backend/formwidgets/ColorPicker.php
+++ b/modules/backend/formwidgets/ColorPicker.php
@@ -57,10 +57,20 @@ class ColorPicker extends FormWidgetBase
      * formats.
      * Allowed values: 'cmyk', 'hex', 'hsl', 'rgb', 'all'
      */
     public $formats = 'hex';
 
+    /**
+     * @var array|string[] Patterns to validate colour string on save
+     */
+    protected array $validationPatterns = [
+        'cmyk' => '/^cmyk\((\d{1,2}\.?\d{0,2}%,? ?){4}\)$/',
+        'hex' => '/^#[\w\d]{6}$/',
+        'hsl' => '/^hsla\((\d{1,3}\.?\d{0,2}%?, ?){3}\d\.?\d{0,2}?\)$/',
+        'rgb' => '/^rgba\((\d{1,3}\.?\d{0,2}, ?){3}\d\.?\d{0,2}?\)$/',
+    ];
+
     //
     // Object properties
     //
 
     /**
@@ -242,8 +252,35 @@ protected function loadAssets()
     /**
      * @inheritDoc
      */
     public function getSaveValue($value)
     {
-        return strlen($value) ? $value : null;
+        if (!strlen($value)) {
+            return null;
+        }
+
+        switch (is_array($this->formats) ? 'all' : $this->formats) {
+            case 'cmyk':
+            case 'hex':
+            case 'hsl':
+            case 'rgb':
+                if (!preg_match($this->validationPatterns[$this->formats], $value)) {
+                    throw new ApplicationException(Lang::get('backend::lang.field.colors_invalid_input'));
+                }
+                break;
+            case 'all':
+                $valid = false;
+                foreach ($this->validationPatterns as $pattern) {
+                    if (preg_match($pattern, $value)) {
+                        $valid = true;
+                        break;
+                    }
+                }
+                if (!$valid) {
+                    throw new ApplicationException(Lang::get('backend::lang.field.colors_invalid_input'));
+                }
+                break;
+        }
+
+        return $value;
     }
 }
diff --git a/modules/backend/lang/en/lang.php b/modules/backend/lang/en/lang.php
index c1842980e6..6479035d50 100644
--- a/modules/backend/lang/en/lang.php
+++ b/modules/backend/lang/en/lang.php
@@ -9,10 +9,11 @@
         'invalid_type' => 'Invalid field type used :type.',
         'options_method_invalid_model' => "The attribute ':field' does not resolve to a valid model. Try specifying the options method for model class :model explicitly.",
         'options_method_not_exists' => "The model class :model must define a method :method() returning options for the ':field' form field.",
         'options_static_method_invalid_value' => "The static method ':method()' on :class did not return a valid options array.",
         'colors_method_not_exists' => "The model class :model must define a method :method() returning html color HEX codes for the ':field' form field.",
+        'colors_invalid_input' => 'The color value supplied is invalid, please try again.',
     ],
     'widget' => [
         'not_registered' => "A widget class name ':name' has not been registered",
         'not_bound' => "A widget with class name ':name' has not been bound to the controller",
     ],
diff --git a/modules/backend/tests/formwidgets/ColorPickerTest.php b/modules/backend/tests/formwidgets/ColorPickerTest.php
new file mode 100644
index 0000000000..c97200441a
--- /dev/null
+++ b/modules/backend/tests/formwidgets/ColorPickerTest.php
@@ -0,0 +1,128 @@
+<?php
+
+namespace Backend\Tests\FormWidgets;
+
+use Backend\Classes\Controller;
+use Backend\Classes\FormField;
+use Backend\FormWidgets\ColorPicker;
+use System\Tests\Bootstrap\PluginTestCase;
+use Winter\Storm\Exception\ApplicationException;
+
+class ColorPickerTest extends PluginTestCase
+{
+    public function testDefaultSaveValue(): void
+    {
+        $widget = $this->makeWidget();
+
+        // Default only expects hex
+        $this->assertEquals('#3498DB', $widget->getSaveValue('#3498DB'));
+
+        // Getting a non-hex value should throw an exception
+        $this->expectException(ApplicationException::class);
+        $widget->getSaveValue('rgba(51.9, 152, 219, 1)');
+
+        // Test a bunch of hex values
+        $this->assertEquals('#3498DB', $widget->getSaveValue('#3498DB'));
+        $this->assertEquals('#2980B9', $widget->getSaveValue('#2980B9'));
+        $this->assertEquals('#9B59B6', $widget->getSaveValue('#9B59B6'));
+    }
+
+    public function testRgbSaveValue(): void
+    {
+        $widget = $this->makeWidget([
+            'formats' => 'rgb'
+        ]);
+
+        // Config specifies only rgb
+        $this->assertEquals('rgba(51.9, 152, 219, 1)', $widget->getSaveValue('rgba(51.9, 152, 219, 1)'));
+
+        // Getting a non-rgb value should throw an exception
+        $this->expectException(ApplicationException::class);
+        $widget->getSaveValue('#3498DB');
+
+        // Test a bunch of rgb values
+        $this->assertEquals('rgba(1, 1, 1, 1)', $widget->getSaveValue('rgba(1, 1, 1, 1)'));
+        $this->assertEquals('rgba(155, 89, 182, 0.5)', $widget->getSaveValue('rgba(155, 89, 182, 0.5)'));
+        $this->assertEquals('rgba(1, 89, 182, 0.55)', $widget->getSaveValue('rgba(1, 89, 182, 0.55)'));
+    }
+
+    public function testCmykSaveValue(): void
+    {
+        $widget = $this->makeWidget([
+            'formats' => 'cmyk'
+        ]);
+
+        // Config specifies only cmyk
+        $this->assertEquals('cmyk(76.3%, 30.6%, 0%, 14.1%)', $widget->getSaveValue('cmyk(76.3%, 30.6%, 0%, 14.1%)'));
+
+        // Getting a non-cmyk value should throw an exception
+        $this->expectException(ApplicationException::class);
+        $widget->getSaveValue('#3498DB');
+
+        // Test a bunch of cmyk values
+        $this->assertEquals('cmyk(14.8%, 51.1%, 0%, 28.6%)', $widget->getSaveValue('cmyk(14.8%, 51.1%, 0%, 28.6%)'));
+        $this->assertEquals('cmyk(17%, 60.75%, 0%, 32.22%)', $widget->getSaveValue('cmyk(17%, 60.75%, 0%, 32.22%)'));
+        $this->assertEquals('cmyk(17.9%, 60.75%, 0%, 32.2%)', $widget->getSaveValue('cmyk(17.9%, 60.75%, 0%, 32.2%)'));
+    }
+
+    public function testHslaSaveValue(): void
+    {
+        $widget = $this->makeWidget([
+            'formats' => 'hsl'
+        ]);
+
+        // Config specifies only hsl
+        $this->assertEquals('hsla(204.1, 69.9%, 53.1%, 1)', $widget->getSaveValue('hsla(204.1, 69.9%, 53.1%, 1)'));
+
+        // Getting a non-hsl value should throw an exception
+        $this->expectException(ApplicationException::class);
+        $widget->getSaveValue('#3498DB');
+
+        // Test a bunch of hsl values
+        $this->assertEquals('hsla(282.3, 43.6%, 47.2%, 1)', $widget->getSaveValue('hsla(282.3, 43.6%, 47.2%, 1)'));
+        $this->assertEquals('hsla(282.3, 43.6%, 47.2%, 0.1)', $widget->getSaveValue('hsla(282.3, 43.6%, 47.2%, 0.1)'));
+        $this->assertEquals('hsla(282, 43.6%, 47.2%, 0.1)', $widget->getSaveValue('hsla(282, 43.6%, 47.2%, 0.1)'));
+        $this->assertEquals('hsla(282, 43.56%, 47.2%, 0.1)', $widget->getSaveValue('hsla(282, 43.56%, 47.2%, 0.1)'));
+        $this->assertEquals('hsla(282.22, 43%, 47.2%, 0.1)', $widget->getSaveValue('hsla(282.22, 43%, 47.2%, 0.1)'));
+    }
+
+    public function testAllSaveValue(): void
+    {
+        $widget = $this->makeWidget([
+            'formats' => 'all'
+        ]);
+
+        // Config allows for any valid format
+        $this->assertEquals('#3498DB', $widget->getSaveValue('#3498DB'));
+        $this->assertEquals('rgba(51.9, 152, 219, 1)', $widget->getSaveValue('rgba(51.9, 152, 219, 1)'));
+        $this->assertEquals('cmyk(76.3%, 30.6%, 0%, 14.1%)', $widget->getSaveValue('cmyk(76.3%, 30.6%, 0%, 14.1%)'));
+        $this->assertEquals('hsla(204.1, 69.9%, 53.1%, 1)', $widget->getSaveValue('hsla(204.1, 69.9%, 53.1%, 1)'));
+
+        // Getting a invalid value should throw an exception
+        $this->expectException(ApplicationException::class);
+        $widget->getSaveValue('#Winter Is Awesome');
+
+        $this->expectException(ApplicationException::class);
+        $widget->getSaveValue('rgba(51.9, 152, 219, 1) -- test');
+
+        $this->expectException(ApplicationException::class);
+        $widget->getSaveValue('Test(51.9, 152, 219, 1)');
+    }
+
+    public function testAllowCustomSaveValue(): void
+    {
+        $widget = $this->makeWidget([
+            'formats' => 'custom'
+        ]);
+
+        // Config allows for any format
+        $this->assertEquals('rgba(51.9, 152, 219, 1)', $widget->getSaveValue('rgba(51.9, 152, 219, 1)'));
+        $this->assertEquals('#Winter Is Awesome', $widget->getSaveValue('#Winter Is Awesome'));
+        $this->assertEquals('Test(51.9, 152, 219, 1)', $widget->getSaveValue('Test(51.9, 152, 219, 1)'));
+    }
+
+    protected function makeWidget(array $config = []): ColorPicker
+    {
+        return new ColorPicker(new Controller(), new FormField('test', 'Test'), $config);
+    }
+}

Vulnerable code:         return strlen($value) ? $value : null;
Fixed code:     /**
     * @var array|string[] Patterns to validate colour string on save
     */
    protected array $validationPatterns = [
        'cmyk' => '/^cmyk\((\d{1,2}\.?\d{0,2}%,? ?){4}\)$/',
        'hex' => '/^#[\w\d]{6}$/',
        'hsl' => '/^hsla\((\d{1,3}\.?\d{0,2}%?, ?){3}\d\.?\d{0,2}?\)$/',
        'rgb' => '/^rgba\((\d{1,3}\.?\d{0,2}, ?){3}\d\.?\d{0,2}?\)$/',
    ];

        if (!strlen($value)) {
            return null;
        }

        switch (is_array($this->formats) ? 'all' : $this->formats) {
            case 'cmyk':
            case 'hex':
            case 'hsl':
            case 'rgb':
                if (!preg_match($this->validationPatterns[$this->formats], $value)) {
                    throw new ApplicationException(Lang::get('backend::lang.field.colors_invalid_input'));
                }
                break;
            case 'all':
                $valid = false;
                foreach ($this->validationPatterns as $pattern) {
                    if (preg_match($pattern, $value)) {
                        $valid = true;
                        break;
                    }
                }
                if (!$valid) {
                    throw new ApplicationException(Lang::get('backend::lang.field.colors_invalid_input'));
                }
                break;
        }

        return $value;
        'colors_invalid_input' => 'The color value supplied is invalid, please try again.',
<?php

namespace Backend\Tests\FormWidgets;

use Backend\Classes\Controller;
use Backend\Classes\FormField;
use Backend\FormWidgets\ColorPicker;
use System\Tests\Bootstrap\PluginTestCase;
use Winter\Storm\Exception\ApplicationException;

class ColorPickerTest extends PluginTestCase
{
    public function testDefaultSaveValue(): void
    {
        $widget = $this->makeWidget();

        // Default only expects hex
        $this->assertEquals('#3498DB', $widget->getSaveValue('#3498DB'));

        // Getting a non-hex value should throw an exception
        $this->expectException(ApplicationException::class);
        $widget->getSaveValue('rgba(51.9, 152, 219, 1)');

        // Test a bunch of hex values
        $this->assertEquals('#3498DB', $widget->getSaveValue('#3498DB'));
        $this->assertEquals('#2980B9', $widget->getSaveValue('#2980B9'));
        $this->assertEquals('#9B59B6', $widget->getSaveValue('#9B59B6'));
    }

    public function testRgbSaveValue(): void
    {
        $widget = $this->makeWidget([
            'formats' => 'rgb'
        ]);

        // Config specifies only rgb
        $this->assertEquals('rgba(51.9, 152, 219, 1)', $widget->getSaveValue('rgba(51.9, 152, 219, 1)'));

        // Getting a non-rgb value should throw an exception
        $this->expectException(ApplicationException::class);
        $widget->getSaveValue('#3498DB');

        // Test a bunch of rgb values
        $this->assertEquals('rgba(1, 1, 1, 1)', $widget->getSaveValue('rgba(1, 1, 1, 1)'));
        $this->assertEquals('rgba(155, 89, 182, 0.5)', $widget->getSaveValue('rgba(155, 89, 182, 0.5)'));
        $this->assertEquals('rgba(1, 89, 182, 0.55)', $widget->getSaveValue('rgba(1, 89, 182, 0.55)'));
    }

    public function testCmykSaveValue(): void
    {
        $widget = $this->makeWidget([
            'formats' => 'cmyk'
        ]);

        // Config specifies only cmyk
        $this->assertEquals('cmyk(76.3%, 30.6%, 0%, 14.1%)', $widget->getSaveValue('cmyk(76.3%, 30.6%, 0%, 14.1%)'));

        // Getting a non-cmyk value should throw an exception
        $this->expectException(ApplicationException::class);
        $widget->getSaveValue('#3498DB');

        // Test a bunch of cmyk values
        $this->assertEquals('cmyk(14.8%, 51.1%, 0%, 28.6%)', $widget->getSaveValue('cmyk(14.8%, 51.1%, 0%, 28.6%)'));
        $this->assertEquals('cmyk(17%, 60.75%, 0%, 32.22%)', $widget->getSaveValue('cmyk(17%, 60.75%, 0%, 32.22%)'));
        $this->assertEquals('cmyk(17.9%, 60.75%, 0%, 32.2%)', $widget->getSaveValue('cmyk(17.9%, 60.75%, 0%, 32.2%)'));
    }

    public function testHslaSaveValue(): void
    {
        $widget = $this->makeWidget([
            'formats' => 'hsl'
        ]);

        // Config specifies only hsl
        $this->assertEquals('hsla(204.1, 69.9%, 53.1%, 1)', $widget->getSaveValue('hsla(204.1, 69.9%, 53.1%, 1)'));

        // Getting a non-hsl value should throw an exception
        $this->expectException(ApplicationException::class);
        $widget->getSaveValue('#3498DB');

        // Test a bunch of hsl values
        $this->assertEquals('hsla(282.3, 43.6%, 47.2%, 1)', $widget->getSaveValue('hsla(282.3, 43.6%, 47.2%, 1)'));
        $this->assertEquals('hsla(282.3, 43.6%, 47.2%, 0.1)', $widget->getSaveValue('hsla(282.3, 43.6%, 47.2%, 0.1)'));
        $this->assertEquals('hsla(282, 43.6%, 47.2%, 0.1)', $widget->getSaveValue('hsla(282, 43.6%, 47.2%, 0.1)'));
        $this->assertEquals('hsla(282, 43.56%, 47.2%, 0.1)', $widget->getSaveValue('hsla(282, 43.56%, 47.2%, 0.1)'));
        $this->assertEquals('hsla(282.22, 43%, 47.2%, 0.1)', $widget->getSaveValue('hsla(282.22, 43%, 47.2%, 0.1)'));
    }

    public function testAllSaveValue(): void
    {
        $widget = $this->makeWidget([
            'formats' => 'all'
        ]);

        // Config allows for any valid format
        $this->assertEquals('#3498DB', $widget->getSaveValue('#3498DB'));
        $this->assertEquals('rgba(51.9, 152, 219, 1)', $widget->getSaveValue('rgba(51.9, 152, 219, 1)'));
        $this->assertEquals('cmyk(76.3%, 30.6%, 0%, 14.1%)', $widget->getSaveValue('cmyk(76.3%, 30.6%, 0%, 14.1%)'));
        $this->assertEquals('hsla(204.1, 69.9%, 53.1%, 1)', $widget->getSaveValue('hsla(204.1, 69.9%, 53.1%, 1)'));

        // Getting a invalid value should throw an exception
        $this->expectException(ApplicationException::class);
        $widget->getSaveValue('#Winter Is Awesome');

        $this->expectException(ApplicationException::class);
        $widget->getSaveValue('rgba(51.9, 152, 219, 1) -- test');

        $this->expectException(ApplicationException::class);
        $widget->getSaveValue('Test(51.9, 152, 219, 1)');
    }

    public function testAllowCustomSaveValue(): void
    {
        $widget = $this->makeWidget([
            'formats' => 'custom'
        ]);

        // Config allows for any format
        $this->assertEquals('rgba(51.9, 152, 219, 1)', $widget->getSaveValue('rgba(51.9, 152, 219, 1)'));
        $this->assertEquals('#Winter Is Awesome', $widget->getSaveValue('#Winter Is Awesome'));
        $this->assertEquals('Test(51.9, 152, 219, 1)', $widget->getSaveValue('Test(51.9, 152, 219, 1)'));
    }

    protected function makeWidget(array $config = []): ColorPicker
    {
        return new ColorPicker(new Controller(), new FormField('test', 'Test'), $config);
    }
}