CVE-2023-4624: Server-Side Request Forgery (SSRF) (CWE-918)
value: Server-Side Request Forgery (SSRF) in GitHub repository bookstackapp/bookstack prior to v23.08.
CWE description: The web server receives a URL or similar request from an upstream component and retrieves the contents of this URL, but it does not sufficiently ensure that the request is being sent to the expected destination.
CVSS v3 base score: 2.4
Published: 2023-08-30T13:15Z
Repository: https://github.com/bookstackapp/bookstack
Language: PHP
Files changed: app/Activity/DispatchWebhookJob.php, app/Config/app.php, app/Util/SsrUrlValidator.php, lang/en/errors.php, tests/Unit/SsrUrlValidatorTest.php
Commit date: 2023-08-26 15:28:29 +0100
Commit message: Security: Added new SSR allow list and validator

Included unit tests to cover validator functionality.
Added to webhooks.
Still need to do testing specifically for webhooks.

Diff: diff --git a/app/Activity/DispatchWebhookJob.php b/app/Activity/DispatchWebhookJob.php
index f2330c4faf..405bca49cb 100644
--- a/app/Activity/DispatchWebhookJob.php
+++ b/app/Activity/DispatchWebhookJob.php
@@ -6,10 +6,11 @@ use BookStack\Activity\Models\Loggable;
 use BookStack\Activity\Models\Webhook;
 use BookStack\Activity\Tools\WebhookFormatter;
 use BookStack\Facades\Theme;
 use BookStack\Theming\ThemeEvents;
 use BookStack\Users\Models\User;
+use BookStack\Util\SsrUrlValidator;
 use Illuminate\Bus\Queueable;
 use Illuminate\Contracts\Queue\ShouldQueue;
 use Illuminate\Foundation\Bus\Dispatchable;
 use Illuminate\Queue\InteractsWithQueue;
 use Illuminate\Queue\SerializesModels;
@@ -51,10 +52,12 @@ class DispatchWebhookJob implements ShouldQueue
     public function handle()
     {
         $lastError = null;
 
         try {
+            (new SsrUrlValidator())->ensureAllowed($this->webhook->endpoint);
+
             $response = Http::asJson()
                 ->withOptions(['allow_redirects' => ['strict' => true]])
                 ->timeout($this->webhook->timeout)
                 ->post($this->webhook->endpoint, $this->webhookData);
         } catch (\Exception $exception) {
diff --git a/app/Config/app.php b/app/Config/app.php
index 29ab9c6dc9..3a843c512b 100644
--- a/app/Config/app.php
+++ b/app/Config/app.php
@@ -64,10 +64,19 @@ return [
     // Can be set to a lone "*" to allow all sources for iframe content (Not advised).
     // Defaults to a set of common services.
     // Current host and source for the "DRAWIO" setting will be auto-appended to the sources configured.
     'iframe_sources' => env('ALLOWED_IFRAME_SOURCES', 'https://*.draw.io https://*.youtube.com https://*.youtube-nocookie.com https://*.vimeo.com'),
 
+    // A list of the sources/hostnames that can be reached by application SSR calls.
+    // This is used wherever users can provide URLs/hosts in-platform, like for webhooks.
+    // Host-specific functionality (usually controlled via other options) like auth
+    // or user avatars for example, won't use this list.
+    // Space seperated if multiple. Can use '*' as a wildcard.
+    // Values will be compared prefix-matched, case-insensitive, against called SSR urls.
+    // Defaults to allow all hosts.
+    'ssr_hosts' => env('ALLOWED_SSR_HOSTS', '*'),
+
     // Alter the precision of IP addresses stored by BookStack.
     // Integer value between 0 (IP hidden) to 4 (Full IP usage)
     'ip_address_precision' => env('IP_ADDRESS_PRECISION', 4),
 
     // Application timezone for back-end date functions.
diff --git a/app/Util/SsrUrlValidator.php b/app/Util/SsrUrlValidator.php
new file mode 100644
index 0000000000..722a45f7b6
--- /dev/null
+++ b/app/Util/SsrUrlValidator.php
@@ -0,0 +1,64 @@
+<?php
+
+namespace BookStack\Util;
+
+use BookStack\Exceptions\HttpFetchException;
+
+class SsrUrlValidator
+{
+    protected string $config;
+
+    public function __construct(string $config = null)
+    {
+        $this->config = $config ?? config('app.ssr_hosts') ?? '';
+    }
+
+    /**
+     * @throws HttpFetchException
+     */
+    public function ensureAllowed(string $url): void
+    {
+        if (!$this->allowed($url)) {
+            throw new HttpFetchException(trans('errors.http_ssr_url_no_match'));
+        }
+    }
+
+    /**
+     * Check if the given URL is allowed by the configured SSR host values.
+     */
+    public function allowed(string $url): bool
+    {
+        $allowed = $this->getHostPatterns();
+
+        foreach ($allowed as $pattern) {
+            if ($this->urlMatchesPattern($url, $pattern)) {
+                return true;
+            }
+        }
+
+        return false;
+    }
+
+    protected function urlMatchesPattern($url, $pattern): bool
+    {
+        $pattern = trim($pattern);
+        $url = trim($url);
+
+        if (empty($pattern) || empty($url)) {
+            return false;
+        }
+
+        $quoted = preg_quote($pattern, '/');
+        $regexPattern = str_replace('\*', '.*', $quoted);
+
+        return preg_match('/^' . $regexPattern . '.*$/i', $url);
+    }
+
+    /**
+     * @return string[]
+     */
+    protected function getHostPatterns(): array
+    {
+        return explode(' ', strtolower($this->config));
+    }
+}
diff --git a/lang/en/errors.php b/lang/en/errors.php
index 23c326f9e0..4cde4cea36 100644
--- a/lang/en/errors.php
+++ b/lang/en/errors.php
@@ -109,6 +109,8 @@ return [
     'api_user_token_expired' => 'The authorization token used has expired',
 
     // Settings & Maintenance
     'maintenance_test_email_failure' => 'Error thrown when sending a test email:',
 
+    // HTTP errors
+    'http_ssr_url_no_match' => 'The URL does not match the configured allowed SSR hosts',
 ];
diff --git a/tests/Unit/SsrUrlValidatorTest.php b/tests/Unit/SsrUrlValidatorTest.php
new file mode 100644
index 0000000000..1443eedd7f
--- /dev/null
+++ b/tests/Unit/SsrUrlValidatorTest.php
@@ -0,0 +1,59 @@
+<?php
+
+namespace Tests\Unit;
+
+use BookStack\Exceptions\HttpFetchException;
+use BookStack\Util\SsrUrlValidator;
+use Tests\TestCase;
+
+class SsrUrlValidatorTest extends TestCase
+{
+    public function test_allowed()
+    {
+        $testMap = [
+            // 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' => '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');
+    }
+}

Fixed code: use BookStack\Util\SsrUrlValidator;
            (new SsrUrlValidator())->ensureAllowed($this->webhook->endpoint);

    // A list of the sources/hostnames that can be reached by application SSR calls.
    // This is used wherever users can provide URLs/hosts in-platform, like for webhooks.
    // Host-specific functionality (usually controlled via other options) like auth
    // or user avatars for example, won't use this list.
    // Space seperated if multiple. Can use '*' as a wildcard.
    // Values will be compared prefix-matched, case-insensitive, against called SSR urls.
    // Defaults to allow all hosts.
    'ssr_hosts' => env('ALLOWED_SSR_HOSTS', '*'),

<?php

namespace BookStack\Util;

use BookStack\Exceptions\HttpFetchException;

class SsrUrlValidator
{
    protected string $config;

    public function __construct(string $config = null)
    {
        $this->config = $config ?? config('app.ssr_hosts') ?? '';
    }

    /**
     * @throws HttpFetchException
     */
    public function ensureAllowed(string $url): void
    {
        if (!$this->allowed($url)) {
            throw new HttpFetchException(trans('errors.http_ssr_url_no_match'));
        }
    }

    /**
     * Check if the given URL is allowed by the configured SSR host values.
     */
    public function allowed(string $url): bool
    {
        $allowed = $this->getHostPatterns();

        foreach ($allowed as $pattern) {
            if ($this->urlMatchesPattern($url, $pattern)) {
                return true;
            }
        }

        return false;
    }

    protected function urlMatchesPattern($url, $pattern): bool
    {
        $pattern = trim($pattern);
        $url = trim($url);

        if (empty($pattern) || empty($url)) {
            return false;
        }

        $quoted = preg_quote($pattern, '/');
        $regexPattern = str_replace('\*', '.*', $quoted);

        return preg_match('/^' . $regexPattern . '.*$/i', $url);
    }

    /**
     * @return string[]
     */
    protected function getHostPatterns(): array
    {
        return explode(' ', strtolower($this->config));
    }
}
    // HTTP errors
    'http_ssr_url_no_match' => 'The URL does not match the configured allowed SSR hosts',
<?php

namespace Tests\Unit;

use BookStack\Exceptions\HttpFetchException;
use BookStack\Util\SsrUrlValidator;
use Tests\TestCase;

class SsrUrlValidatorTest extends TestCase
{
    public function test_allowed()
    {
        $testMap = [
            // 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' => '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');
    }
}