CVE-2023-22729: URL Redirection to Untrusted Site ('Open Redirect') (CWE-601)
value: Silverstripe Framework is the Model-View-Controller framework that powers the Silverstripe content management system. Prior to version 4.12.15, an attacker can display a link to a third party website on a login screen by convincing a legitimate content author to follow a specially crafted link. Users should upgrade to Silverstripe Framework 4.12.15 or above to address the issue.
CWE description: A web application accepts a user-controlled input that specifies a link to an external site, and uses that link in a Redirect. This simplifies phishing attacks.
CVSS v3 base score: 6.1
Published: 2023-04-26T15:15Z
Repository: https://github.com/silverstripe/silverstripe-framework
Language: PHP
Files changed: src/Control/Director.php, tests/php/Control/DirectorTest.php
Commit date: 2023-04-26 09:49:59 +1200
Commit message: [CVE-2023-22729] Escaped double slash is absolute URL

Diff: diff --git a/src/Control/Director.php b/src/Control/Director.php
index 91023b7a76..ee7266bd47 100644
--- a/src/Control/Director.php
+++ b/src/Control/Director.php
@@ -820,14 +820,15 @@ class Director implements TemplateGlobalProvider
         $colonPosition = strpos($url ?? '', ':');
         $slashPosition = strpos($url ?? '', '/');
         return (
             // Base check for existence of a host on a compliant URL
             parse_url($url ?? '', PHP_URL_HOST)
-            // Check for more than one leading slash without a protocol.
+            // Check for more than one leading slash (forward or backward) without a protocol.
             // While not a RFC compliant absolute URL, it is completed to a valid URL by some browsers,
             // and hence a potential security risk. Single leading slashes are not an issue though.
-            || preg_match('%^\s*/{2,}%', $url ?? '')
+            // Note: Need 4 backslashes to have a single non-escaped backslash for regex.
+            || preg_match('%^\s*(\\\\|/){2,}%', $url ?? '')
             || (
                 // If a colon is found, check if it's part of a valid scheme definition
                 // (meaning its not preceded by a slash).
                 $colonPosition !== false
                 && ($slashPosition === false || $colonPosition < $slashPosition)
diff --git a/tests/php/Control/DirectorTest.php b/tests/php/Control/DirectorTest.php
index 05e24046bd..b8be9ec465 100644
--- a/tests/php/Control/DirectorTest.php
+++ b/tests/php/Control/DirectorTest.php
@@ -230,19 +230,27 @@ class DirectorTest extends SapphireTest
     }
 
     public function testIsAbsoluteUrl()
     {
         $this->assertTrue(Director::is_absolute_url('http://test.com/testpage'));
+        $this->assertTrue(Director::is_absolute_url('https:/\\test.com'));
+        $this->assertTrue(Director::is_absolute_url('https:\\/test.com'));
+        $this->assertTrue(Director::is_absolute_url('https:\\\\test.com'));
         $this->assertTrue(Director::is_absolute_url('ftp://test.com'));
         $this->assertFalse(Director::is_absolute_url('test.com/testpage'));
         $this->assertFalse(Director::is_absolute_url('/relative'));
         $this->assertFalse(Director::is_absolute_url('relative'));
         $this->assertFalse(Director::is_absolute_url("/relative/?url=http://foo.com"));
         $this->assertFalse(Director::is_absolute_url("/relative/#http://foo.com"));
         $this->assertTrue(Director::is_absolute_url("https://test.com/?url=http://foo.com"));
         $this->assertTrue(Director::is_absolute_url("trickparseurl:http://test.com"));
         $this->assertTrue(Director::is_absolute_url('//test.com'));
+        $this->assertTrue(Director::is_absolute_url('\\/\\/test.com'));
+        $this->assertTrue(Director::is_absolute_url('\/\/test.com'));
+        $this->assertTrue(Director::is_absolute_url('/\\test.com'));
+        $this->assertTrue(Director::is_absolute_url('\\\\test.com'));
+        $this->assertFalse(Director::is_absolute_url('\\test.com'));
         $this->assertTrue(Director::is_absolute_url('/////test.com'));
         $this->assertTrue(Director::is_absolute_url('  ///test.com'));
         $this->assertTrue(Director::is_absolute_url('http:test.com'));
         $this->assertTrue(Director::is_absolute_url('//http://test.com'));
     }
@@ -256,12 +264,21 @@ class DirectorTest extends SapphireTest
 
     public function testIsRelativeUrl()
     {
         $this->assertFalse(Director::is_relative_url('http://test.com'));
         $this->assertFalse(Director::is_relative_url('https://test.com'));
+        $this->assertFalse(Director::is_relative_url('https:/\\test.com'));
+        $this->assertFalse(Director::is_relative_url('https:\\/test.com'));
+        $this->assertFalse(Director::is_relative_url('https:\\\\test.com'));
         $this->assertFalse(Director::is_relative_url('   https://test.com/testpage   '));
         $this->assertTrue(Director::is_relative_url('test.com/testpage'));
+        $this->assertFalse(Director::is_relative_url('//test.com'));
+        $this->assertFalse(Director::is_relative_url('\\/\\/test.com'));
+        $this->assertFalse(Director::is_relative_url('\/\/test.com'));
+        $this->assertFalse(Director::is_relative_url('/\\test.com'));
+        $this->assertFalse(Director::is_relative_url('\\\\test.com'));
+        $this->assertTrue(Director::is_relative_url('\\test.com'));
         $this->assertFalse(Director::is_relative_url('ftp://test.com'));
         $this->assertTrue(Director::is_relative_url('/relative'));
         $this->assertTrue(Director::is_relative_url('relative'));
         $this->assertTrue(Director::is_relative_url('/relative/?url=http://test.com'));
         $this->assertTrue(Director::is_relative_url('/relative/#=http://test.com'));
@@ -399,21 +416,38 @@ class DirectorTest extends SapphireTest
             $actualRelative,
             "Expected relativeURL of {$requestURL} to be {$relativeURL}"
         );
     }
 
-    /**
-     * Mostly tested by {@link testIsRelativeUrl()},
-     * just adding the host name matching aspect here.
-     */
     public function testIsSiteUrl()
     {
-        $this->assertFalse(Director::is_site_url("http://test.com"));
+        $this->assertFalse(Director::is_site_url('http://test.com'));
+        $this->assertTrue(Director::is_site_url('/relative-path'));
+        $this->assertTrue(Director::is_site_url('relative-path'));
         $this->assertTrue(Director::is_site_url(Director::absoluteBaseURL()));
-        $this->assertFalse(Director::is_site_url("http://test.com?url=" . Director::absoluteBaseURL()));
-        $this->assertFalse(Director::is_site_url("http://test.com?url=" . urlencode(Director::absoluteBaseURL() ?? '')));
-        $this->assertFalse(Director::is_site_url("//test.com?url=" . Director::absoluteBaseURL()));
+        $this->assertFalse(Director::is_site_url('http://test.com?url=' . Director::absoluteBaseURL()));
+        $this->assertFalse(Director::is_site_url('http://test.com?url=' . urlencode(Director::absoluteBaseURL() ?? '')));
+        $this->assertFalse(Director::is_site_url('http:\\\\test.com'));
+        $this->assertFalse(Director::is_site_url('http:\\\\test.com?url=' . Director::absoluteBaseURL()));
+        $this->assertFalse(Director::is_site_url('http:\\\\test.com?url=' . urlencode(Director::absoluteBaseURL() ?? '')));
+        $this->assertFalse(Director::is_site_url('http:\\/test.com'));
+        $this->assertFalse(Director::is_site_url('http:\\/test.com?url=' . Director::absoluteBaseURL()));
+        $this->assertFalse(Director::is_site_url('http:\\/test.com?url=' . urlencode(Director::absoluteBaseURL() ?? '')));
+        $this->assertFalse(Director::is_site_url('//test.com'));
+        $this->assertFalse(Director::is_site_url('//test.com?url=' . Director::absoluteBaseURL()));
+        $this->assertFalse(Director::is_site_url('\\/\\/test.com'));
+        $this->assertFalse(Director::is_site_url('\\/\\/test.com?url=' . Director::absoluteBaseURL()));
+        $this->assertFalse(Director::is_site_url('\/\/test.com'));
+        $this->assertFalse(Director::is_site_url('\/\/test.com?url=' . Director::absoluteBaseURL()));
+        $this->assertFalse(Director::is_site_url('\\/test.com'));
+        $this->assertFalse(Director::is_site_url('\\/test.com?url=' . Director::absoluteBaseURL()));
+        $this->assertFalse(Director::is_site_url('/\\test.com'));
+        $this->assertFalse(Director::is_site_url('/\\test.com?url=' . Director::absoluteBaseURL()));
+        $this->assertFalse(Director::is_site_url('\\\\test.com'));
+        $this->assertFalse(Director::is_site_url('\\\\test.com?url=' . Director::absoluteBaseURL()));
+        $this->assertTrue(Director::is_site_url('\\test.com'));
+        $this->assertTrue(Director::is_site_url('\\test.com?url=' . Director::absoluteBaseURL()));
         $this->assertFalse(Director::is_site_url('http://google.com\@test.com'));
         $this->assertFalse(Director::is_site_url('http://google.com/@test.com'));
         $this->assertFalse(Director::is_site_url('http://google.com:pass\@test.com'));
         $this->assertFalse(Director::is_site_url('http://google.com:pass/@test.com'));
     }

Vulnerable code:             // Check for more than one leading slash without a protocol.
            || preg_match('%^\s*/{2,}%', $url ?? '')
    /**
     * Mostly tested by {@link testIsRelativeUrl()},
     * just adding the host name matching aspect here.
     */
        $this->assertFalse(Director::is_site_url("http://test.com"));
        $this->assertFalse(Director::is_site_url("http://test.com?url=" . Director::absoluteBaseURL()));
        $this->assertFalse(Director::is_site_url("http://test.com?url=" . urlencode(Director::absoluteBaseURL() ?? '')));
        $this->assertFalse(Director::is_site_url("//test.com?url=" . Director::absoluteBaseURL()));
Fixed code:             // Check for more than one leading slash (forward or backward) without a protocol.
            // Note: Need 4 backslashes to have a single non-escaped backslash for regex.
            || preg_match('%^\s*(\\\\|/){2,}%', $url ?? '')
        $this->assertTrue(Director::is_absolute_url('https:/\\test.com'));
        $this->assertTrue(Director::is_absolute_url('https:\\/test.com'));
        $this->assertTrue(Director::is_absolute_url('https:\\\\test.com'));
        $this->assertTrue(Director::is_absolute_url('\\/\\/test.com'));
        $this->assertTrue(Director::is_absolute_url('\/\/test.com'));
        $this->assertTrue(Director::is_absolute_url('/\\test.com'));
        $this->assertTrue(Director::is_absolute_url('\\\\test.com'));
        $this->assertFalse(Director::is_absolute_url('\\test.com'));
        $this->assertFalse(Director::is_relative_url('https:/\\test.com'));
        $this->assertFalse(Director::is_relative_url('https:\\/test.com'));
        $this->assertFalse(Director::is_relative_url('https:\\\\test.com'));
        $this->assertFalse(Director::is_relative_url('//test.com'));
        $this->assertFalse(Director::is_relative_url('\\/\\/test.com'));
        $this->assertFalse(Director::is_relative_url('\/\/test.com'));
        $this->assertFalse(Director::is_relative_url('/\\test.com'));
        $this->assertFalse(Director::is_relative_url('\\\\test.com'));
        $this->assertTrue(Director::is_relative_url('\\test.com'));
        $this->assertFalse(Director::is_site_url('http://test.com'));
        $this->assertTrue(Director::is_site_url('/relative-path'));
        $this->assertTrue(Director::is_site_url('relative-path'));
        $this->assertFalse(Director::is_site_url('http://test.com?url=' . Director::absoluteBaseURL()));
        $this->assertFalse(Director::is_site_url('http://test.com?url=' . urlencode(Director::absoluteBaseURL() ?? '')));
        $this->assertFalse(Director::is_site_url('http:\\\\test.com'));
        $this->assertFalse(Director::is_site_url('http:\\\\test.com?url=' . Director::absoluteBaseURL()));
        $this->assertFalse(Director::is_site_url('http:\\\\test.com?url=' . urlencode(Director::absoluteBaseURL() ?? '')));
        $this->assertFalse(Director::is_site_url('http:\\/test.com'));
        $this->assertFalse(Director::is_site_url('http:\\/test.com?url=' . Director::absoluteBaseURL()));
        $this->assertFalse(Director::is_site_url('http:\\/test.com?url=' . urlencode(Director::absoluteBaseURL() ?? '')));
        $this->assertFalse(Director::is_site_url('//test.com'));
        $this->assertFalse(Director::is_site_url('//test.com?url=' . Director::absoluteBaseURL()));
        $this->assertFalse(Director::is_site_url('\\/\\/test.com'));
        $this->assertFalse(Director::is_site_url('\\/\\/test.com?url=' . Director::absoluteBaseURL()));
        $this->assertFalse(Director::is_site_url('\/\/test.com'));
        $this->assertFalse(Director::is_site_url('\/\/test.com?url=' . Director::absoluteBaseURL()));
        $this->assertFalse(Director::is_site_url('\\/test.com'));
        $this->assertFalse(Director::is_site_url('\\/test.com?url=' . Director::absoluteBaseURL()));
        $this->assertFalse(Director::is_site_url('/\\test.com'));
        $this->assertFalse(Director::is_site_url('/\\test.com?url=' . Director::absoluteBaseURL()));
        $this->assertFalse(Director::is_site_url('\\\\test.com'));
        $this->assertFalse(Director::is_site_url('\\\\test.com?url=' . Director::absoluteBaseURL()));
        $this->assertTrue(Director::is_site_url('\\test.com'));
        $this->assertTrue(Director::is_site_url('\\test.com?url=' . Director::absoluteBaseURL()));