From cf648906e94a22f802898ba38236edd09a3909e4 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Thu, 30 Apr 2026 09:31:56 +0100 Subject: [PATCH] SSR: Hardened URL validator against a range of workarounds Added a more comprehensive range of tests to cover. Thanks to naruhodoowl (https://github.com/kilhsrito-crypto) for reporting. --- app/Util/SsrUrlValidator.php | 31 ++++++- tests/Unit/SsrUrlValidatorTest.php | 62 ------------- tests/Util/SsrUrlValidatorTest.php | 142 +++++++++++++++++++++++++++++ 3 files changed, 169 insertions(+), 66 deletions(-) delete mode 100644 tests/Unit/SsrUrlValidatorTest.php create mode 100644 tests/Util/SsrUrlValidatorTest.php diff --git a/app/Util/SsrUrlValidator.php b/app/Util/SsrUrlValidator.php index 076a653fc..a9b5c138d 100644 --- a/app/Util/SsrUrlValidator.php +++ b/app/Util/SsrUrlValidator.php @@ -8,6 +8,10 @@ use BookStack\Exceptions\HttpFetchException; * Validate the host we're connecting to when making a server-side-request. * Will use the given hosts config if given during construction otherwise * will look to the app configured config. + * + * The config format is a space-seperated list of URL prefixes which should contain the + * protocol and host. It can optionally define a path prefix as part of the URL. + * Wildcards, via a '*', can be used within these elements to match anything but a '/'. */ class SsrUrlValidator { @@ -48,15 +52,34 @@ class SsrUrlValidator { $pattern = rtrim(trim($pattern), '/'); $url = trim($url); + $urlParts = parse_url($url); - if (empty($pattern) || empty($url)) { + if (empty($pattern) || empty($url) || $urlParts === false) { return false; } - $quoted = preg_quote($pattern, '/'); - $regexPattern = str_replace('\*', '.*', $quoted); + // Prevent potential tricks using percent encoded slashes + if (str_contains(strtolower($urlParts['host'] ?? ''), '%2f')) { + return false; + } - return preg_match('/^' . $regexPattern . '($|\/.*$|#.*$)/i', $url); + // Disregard query and fragment + $url = explode('?', $url, 2)[0]; + $url = explode('#', $url, 2)[0]; + + // Disregard userinfo if existing + if (!empty($urlParts['user']) || !empty($urlParts['pass'])) { + [$start, $postUserinfo] = explode('@', $url, 2); + $preUserinfo = explode('//', $start, 2)[0]; + $url = ($preUserinfo ? $preUserinfo . '//' : '') . $postUserinfo; + } + + // Prepare pattern + $quoted = preg_quote($pattern, '/'); + $regexPattern = str_replace('\*', '[^\/]*', $quoted); + + // Check against our URL + return preg_match('/^' . $regexPattern . '($|\/.*$)/i', $url); } /** diff --git a/tests/Unit/SsrUrlValidatorTest.php b/tests/Unit/SsrUrlValidatorTest.php deleted file mode 100644 index 8fb538916..000000000 --- a/tests/Unit/SsrUrlValidatorTest.php +++ /dev/null @@ -1,62 +0,0 @@ - '', 'url' => '', 'result' => false], - ['config' => '', 'url' => 'https://example.com', 'result' => false], - ['config' => ' ', 'url' => 'https://example.com', 'result' => false], - ['config' => '*', 'url' => '', 'result' => false], - ['config' => '*', 'url' => 'https://example.com', 'result' => true], - ['config' => 'https://*', 'url' => 'https://example.com', 'result' => true], - ['config' => 'http://*', 'url' => 'https://example.com', 'result' => false], - ['config' => 'https://*example.com', 'url' => 'https://example.com', 'result' => true], - ['config' => 'https://*ample.com', 'url' => 'https://example.com', 'result' => true], - ['config' => 'https://*.example.com', 'url' => 'https://example.com', 'result' => false], - ['config' => 'https://*.example.com', 'url' => 'https://test.example.com', 'result' => true], - ['config' => '*//example.com', 'url' => 'https://example.com', 'result' => true], - ['config' => '*//example.com', 'url' => 'http://example.com', 'result' => true], - ['config' => '*//example.co', 'url' => 'http://example.co.uk', 'result' => false], - ['config' => '*//example.co/bookstack', 'url' => 'https://example.co/bookstack/a/path', 'result' => true], - ['config' => '*//example.co*', 'url' => 'https://example.co.uk/bookstack/a/path', 'result' => true], - ['config' => 'https://example.com', 'url' => 'https://example.com/a/b/c?test=cat', 'result' => true], - ['config' => 'https://example.com', 'url' => 'https://example.co.uk', 'result' => false], - - // Escapes - ['config' => 'https://(.*?).com', 'url' => 'https://example.com', 'result' => false], - ['config' => 'https://example.com', 'url' => 'https://example.co.uk#https://example.com', 'result' => false], - - // Multi values - ['config' => '*//example.org *//example.com', 'url' => 'https://example.com', 'result' => true], - ['config' => '*//example.org *//example.com', 'url' => 'https://example.com/a/b/c?test=cat#hello', 'result' => true], - ['config' => '*.example.org *.example.com', 'url' => 'https://example.co.uk', 'result' => false], - ['config' => ' *.example.org *.example.com ', 'url' => 'https://example.co.uk', 'result' => false], - ['config' => '* *.example.com', 'url' => 'https://example.co.uk', 'result' => true], - ['config' => '*//example.org *//example.com *//example.co.uk', 'url' => 'https://example.co.uk', 'result' => true], - ['config' => '*//example.org *//example.com *//example.co.uk', 'url' => 'https://example.net', 'result' => false], - ]; - - foreach ($testMap as $test) { - $result = (new SsrUrlValidator($test['config']))->allowed($test['url']); - $this->assertEquals($test['result'], $result, "Failed asserting url '{$test['url']}' with config '{$test['config']}' results " . ($test['result'] ? 'true' : 'false')); - } - } - - public function test_enssure_allowed() - { - $result = (new SsrUrlValidator('https://example.com'))->ensureAllowed('https://example.com'); - $this->assertNull($result); - - $this->expectException(HttpFetchException::class); - (new SsrUrlValidator('https://example.com'))->ensureAllowed('https://test.example.com'); - } -} diff --git a/tests/Util/SsrUrlValidatorTest.php b/tests/Util/SsrUrlValidatorTest.php new file mode 100644 index 000000000..12e176517 --- /dev/null +++ b/tests/Util/SsrUrlValidatorTest.php @@ -0,0 +1,142 @@ +set([ + 'app.ssr_hosts' => 'https://donkey.example.com', + ]); + + $validator = new SsrUrlValidator(); + + $this->assertTrue($validator->allowed('https://donkey.example.com')); + $this->assertFalse($validator->allowed('https://monkey.example.com')); + } + + public function test_config_string_can_be_passed_in_constructor() + { + config()->set([ + 'app.ssr_hosts' => 'https://donkey.example.com', + ]); + + $validator = new SsrUrlValidator('https://monkey.example.com'); + + $this->assertFalse($validator->allowed('https://donkey.example.com')); + $this->assertTrue($validator->allowed('https://monkey.example.com')); + } + + public function test_config_string_can_include_multiple_space_seperated_values() + { + $validator = new SsrUrlValidator('https://monkey.example.com https://cat.example.com'); + + $this->assertFalse($validator->allowed('https://donkey.example.com')); + $this->assertTrue($validator->allowed('https://monkey.example.com')); + $this->assertTrue($validator->allowed('https://cat.example.com')); + } + + public function test_ensure_allowed_throws_if_not_allowed() + { + $validator = new SsrUrlValidator('https://monkey.example.com'); + + $this->assertNull($validator->ensureAllowed('https://monkey.example.com')); + + $this->assertThrows(function () use ($validator) { + $validator->ensureAllowed('https://donkey.example.com'); + }, HttpFetchException::class, 'The URL does not match the configured allowed SSR hosts'); + } + + public function test_basic_url_matching() + { + $tests = [ + // Single values + ['config' => '', 'url' => '', 'result' => false], + ['config' => '', 'url' => 'https://example.com', 'result' => false], + ['config' => ' ', 'url' => 'https://example.com', 'result' => false], + ['config' => '*', 'url' => '', 'result' => false], + ['config' => '*', 'url' => 'https://example.com', 'result' => true], + ['config' => 'https://*', 'url' => 'https://example.com', 'result' => true], + ['config' => 'http://*', 'url' => 'https://example.com', 'result' => false], + ['config' => 'https://*example.com', 'url' => 'https://example.com', 'result' => true], + ['config' => 'https://*ample.com', 'url' => 'https://example.com', 'result' => true], + ['config' => 'https://*.example.com', 'url' => 'https://example.com', 'result' => false], + ['config' => 'https://*.example.com', 'url' => 'https://test.example.com', 'result' => true], + ['config' => '*//example.com', 'url' => 'https://example.com', 'result' => true], + ['config' => '*//example.com', 'url' => 'http://example.com', 'result' => true], + ['config' => '*//example.co', 'url' => 'http://example.co.uk', 'result' => false], + ['config' => '*//example.co/bookstack', 'url' => 'https://example.co/bookstack/a/path', 'result' => true], + ['config' => '*//example.co*', 'url' => 'https://example.co.uk/bookstack/a/path', 'result' => true], + ['config' => 'https://example.com', 'url' => 'https://example.com/a/b/c?test=cat', 'result' => true], + ['config' => 'https://example.com', 'url' => 'https://example.co.uk', 'result' => false], + + // Escapes + ['config' => 'https://(.*?).com', 'url' => 'https://example.com', 'result' => false], + ['config' => 'https://example.com', 'url' => 'https://example.co.uk#https://example.com', 'result' => false], + + // Multi values + ['config' => '*//example.org *//example.com', 'url' => 'https://example.com', 'result' => true], + ['config' => '*//example.org *//example.com', 'url' => 'https://example.com/a/b/c?test=cat#hello', 'result' => true], + ['config' => '*.example.org *.example.com', 'url' => 'https://example.co.uk', 'result' => false], + ['config' => ' *.example.org *.example.com ', 'url' => 'https://example.co.uk', 'result' => false], + ['config' => '* *.example.com', 'url' => 'https://example.co.uk', 'result' => true], + ['config' => '*//example.org *//example.com *//example.co.uk', 'url' => 'https://example.co.uk', 'result' => true], + ['config' => '*//example.org *//example.com *//example.co.uk', 'url' => 'https://example.net', 'result' => false], + + // Further tests + ['config' => 'https://monkey.example.com', 'url' => 'https://monkey.example.com/a/b', 'result' => true,], + ['config' => 'https://monkey.example.com', 'url' => 'https://monkey.example.com/a/b?a=b#ab', 'result' => true,], + ['config' => 'https://monkey.example.com', 'url' => 'https://monkey.example.com:8080/a', 'result' => false,], + ['config' => '*', 'url' => 'https://a.example.com', 'result' => true,], + ['config' => 'https://monkey.example.com', 'url' => 'http://monkey.example.com/a/b?a=b#ab', 'result' => false,], + ['config' => 'https://monkey.example.com', 'url' => 'https://beans.monkey.example.com/a/b?a=b#ab', 'result' => false,], + ['config' => 'https://*monkey.example.com', 'url' => 'https://amonkey.example.com/a/b?a=b#ab', 'result' => true,], + ['config' => 'https://*monkey.example.com', 'url' => 'https://donkey.example.com/a/b/monkey.example.com/b?a=b#ab', 'result' => false,], + ['config' => 'https://monkey.example.com', 'url' => 'https://example.com/monkey.example.com/b?a=monkey.example.com#monkey.example.com', 'result' => false,], + ['config' => 'https://*.example.com', 'url' => 'https://a.b.example.com/a/b', 'result' => true,], + ['config' => 'https://*.example.com', 'url' => 'https://a.b.example.a.com/a/b', 'result' => false,], + ['config' => 'https://*.example.com', 'url' => 'https://a.com/a/b?val=a.example.com', 'result' => false,], + ['config' => 'https://*.example.com', 'url' => 'https://a.com/a/b#example.com', 'result' => false,], + ['config' => 'https://a.*.example.com', 'url' => 'https://a.b.c.example.com/c/d', 'result' => true,], + ['config' => 'https://example.com/webhooks/', 'url' => 'https://example.com/webhooks/beans', 'result' => true,], + ['config' => 'https://example.com/webhooks/', 'url' => 'https://example.com/a/webhooks/', 'result' => false,], + ['config' => 'https://example.com:8080', 'url' => 'https://example.com/a/b', 'result' => false,], + ['config' => 'https://example.com:8080', 'url' => 'https://example.com:8080/a/b', 'result' => true,], + ['config' => 'https://example.com/*', 'url' => 'https://example.com:8080/a/b', 'result' => false,], + ]; + + foreach ($tests as $testCase) { + $validator = new SsrUrlValidator($testCase['config']); + $result = $validator->allowed($testCase['url']); + $this->assertEquals($testCase['result'], $result, "Failed asserting expected result for config {$testCase['config']} and test value {$testCase['url']}"); + } + } + + public function test_wildcard_does_not_match_userinfo_data_but_still_allows_it() + { + $validator = new SsrUrlValidator('https://*monkey.example.com'); + $this->assertFalse($validator->allowed('https://monkey.example.com@a.example.com')); + + $validator = new SsrUrlValidator('https://monkey.example.com*'); + $this->assertFalse($validator->allowed('https://monkey.example.com@a.example.com')); + $this->assertFalse($validator->allowed('https://monkey.example.com:monkey.example.com@a.example.com')); + + $validator = new SsrUrlValidator('https://monkey.example.com'); + $this->assertTrue($validator->allowed('https://a:b@monkey.example.com')); + } + + public function test_percent_encoded_slashes_in_host_are_rejected() + { + $validator = new SsrUrlValidator('*'); + + $this->assertFalse($validator->allowed('https://cat.example.com%2Fa/b')); + $this->assertFalse($validator->allowed('https://cat.example.com%2fa/b')); + $this->assertFalse($validator->allowed('https://cat%2f.example.com/a/b')); + $this->assertFalse($validator->allowed('https://cat.exa%2Fmple.com')); + } +}