Skip to content

Commit 642210a

Browse files
ssddanbrownssddanbrown
committed
Merge branch 'srr_host_allowlist' into development
2 parents e176aae + 9038958 commit 642210a

File tree

6 files changed

+154
-0
lines changed

6 files changed

+154
-0
lines changed

‎app/Activity/DispatchWebhookJob.php‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
use BookStack\Facades\Theme;
99
use BookStack\Theming\ThemeEvents;
1010
use BookStack\Users\Models\User;
11+
use BookStack\Util\SsrUrlValidator;
1112
use Illuminate\Bus\Queueable;
1213
use Illuminate\Contracts\Queue\ShouldQueue;
1314
use Illuminate\Foundation\Bus\Dispatchable;
@@ -53,6 +54,8 @@ public function handle()
5354
$lastError = null;
5455

5556
try {
57+
(new SsrUrlValidator())->ensureAllowed($this->webhook->endpoint);
58+
5659
$response = Http::asJson()
5760
->withOptions(['allow_redirects' => ['strict' => true]])
5861
->timeout($this->webhook->timeout)

‎app/Config/app.php‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,15 @@
6666
// Current host and source for the "DRAWIO" setting will be auto-appended to the sources configured.
6767
'iframe_sources' => env('ALLOWED_IFRAME_SOURCES', 'https://*.draw.io https://*.youtube.com https://*.youtube-nocookie.com https://*.vimeo.com'),
6868

69+
// A list of the sources/hostnames that can be reached by application SSR calls.
70+
// This is used wherever users can provide URLs/hosts in-platform, like for webhooks.
71+
// Host-specific functionality (usually controlled via other options) like auth
72+
// or user avatars for example, won't use this list.
73+
// Space seperated if multiple. Can use '*' as a wildcard.
74+
// Values will be compared prefix-matched, case-insensitive, against called SSR urls.
75+
// Defaults to allow all hosts.
76+
'ssr_hosts' => env('ALLOWED_SSR_HOSTS', '*'),
77+
6978
// Alter the precision of IP addresses stored by BookStack.
7079
// Integer value between 0 (IP hidden) to 4 (Full IP usage)
7180
'ip_address_precision' => env('IP_ADDRESS_PRECISION', 4),

‎app/Util/SsrUrlValidator.php‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
<?php
2+
3+
namespace BookStack\Util;
4+
5+
use BookStack\Exceptions\HttpFetchException;
6+
7+
class SsrUrlValidator
8+
{
9+
protected string $config;
10+
11+
public function __construct(string $config = null)
12+
{
13+
$this->config = $config ?? config('app.ssr_hosts') ?? '';
14+
}
15+
16+
/**
17+
* @throws HttpFetchException
18+
*/
19+
public function ensureAllowed(string $url): void
20+
{
21+
if (!$this->allowed($url)) {
22+
throw new HttpFetchException(trans('errors.http_ssr_url_no_match'));
23+
}
24+
}
25+
26+
/**
27+
* Check if the given URL is allowed by the configured SSR host values.
28+
*/
29+
public function allowed(string $url): bool
30+
{
31+
$allowed = $this->getHostPatterns();
32+
33+
foreach ($allowed as $pattern) {
34+
if ($this->urlMatchesPattern($url, $pattern)) {
35+
return true;
36+
}
37+
}
38+
39+
return false;
40+
}
41+
42+
protected function urlMatchesPattern($url, $pattern): bool
43+
{
44+
$pattern = rtrim(trim($pattern), '/');
45+
$url = trim($url);
46+
47+
if (empty($pattern) || empty($url)) {
48+
return false;
49+
}
50+
51+
$quoted = preg_quote($pattern, '/');
52+
$regexPattern = str_replace('\*', '.*', $quoted);
53+
54+
return preg_match('/^' . $regexPattern . '($|\/.*$|#.*$)/i', $url);
55+
}
56+
57+
/**
58+
* @return string[]
59+
*/
60+
protected function getHostPatterns(): array
61+
{
62+
return explode(' ', strtolower($this->config));
63+
}
64+
}

‎lang/en/errors.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,4 +111,6 @@
111111
// Settings & Maintenance
112112
'maintenance_test_email_failure' => 'Error thrown when sending a test email:',
113113

114+
// HTTP errors
115+
'http_ssr_url_no_match' => 'The URL does not match the configured allowed SSR hosts',
114116
];

‎tests/Actions/WebhookCallTest.php‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,20 @@ public function test_webhook_call_exception_is_caught_and_logged()
101101
$this->assertNotNull($webhook->last_errored_at);
102102
}
103103

104+
public function test_webhook_uses_ssr_hosts_option_if_set()
105+
{
106+
config()->set('app.ssr_hosts', 'https://*.example.com');
107+
$http = Http::fake();
108+
109+
$webhook = $this->newWebhook(['active' => true, 'endpoint' => 'https://wh.example.co.uk'], ['all']);
110+
$this->runEvent(ActivityType::ROLE_CREATE);
111+
$http->assertNothingSent();
112+
113+
$webhook->refresh();
114+
$this->assertEquals('The URL does not match the configured allowed SSR hosts', $webhook->last_error);
115+
$this->assertNotNull($webhook->last_errored_at);
116+
}
117+
104118
public function test_webhook_call_data_format()
105119
{
106120
Http::fake([

‎tests/Unit/SsrUrlValidatorTest.php‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
<?php
2+
3+
namespace Tests\Unit;
4+
5+
use BookStack\Exceptions\HttpFetchException;
6+
use BookStack\Util\SsrUrlValidator;
7+
use Tests\TestCase;
8+
9+
class SsrUrlValidatorTest extends TestCase
10+
{
11+
public function test_allowed()
12+
{
13+
$testMap = [
14+
// Single values
15+
['config' => '', 'url' => '', 'result' => false],
16+
['config' => '', 'url' => 'https://example.com', 'result' => false],
17+
['config' => ' ', 'url' => 'https://example.com', 'result' => false],
18+
['config' => '*', 'url' => '', 'result' => false],
19+
['config' => '*', 'url' => 'https://example.com', 'result' => true],
20+
['config' => 'https://*', 'url' => 'https://example.com', 'result' => true],
21+
['config' => 'http://*', 'url' => 'https://example.com', 'result' => false],
22+
['config' => 'https://*example.com', 'url' => 'https://example.com', 'result' => true],
23+
['config' => 'https://*ample.com', 'url' => 'https://example.com', 'result' => true],
24+
['config' => 'https://*.example.com', 'url' => 'https://example.com', 'result' => false],
25+
['config' => 'https://*.example.com', 'url' => 'https://test.example.com', 'result' => true],
26+
['config' => '*//example.com', 'url' => 'https://example.com', 'result' => true],
27+
['config' => '*//example.com', 'url' => 'http://example.com', 'result' => true],
28+
['config' => '*//example.co', 'url' => 'http://example.co.uk', 'result' => false],
29+
['config' => '*//example.co/bookstack', 'url' => 'https://example.co/bookstack/a/path', 'result' => true],
30+
['config' => '*//example.co*', 'url' => 'https://example.co.uk/bookstack/a/path', 'result' => true],
31+
['config' => 'https://example.com', 'url' => 'https://example.com/a/b/c?test=cat', 'result' => true],
32+
['config' => 'https://example.com', 'url' => 'https://example.co.uk', 'result' => false],
33+
34+
// Escapes
35+
['config' => 'https://(.*?).com', 'url' => 'https://example.com', 'result' => false],
36+
['config' => 'https://example.com', 'url' => 'https://example.co.uk#https://example.com', 'result' => false],
37+
38+
// Multi values
39+
['config' => '*//example.org *//example.com', 'url' => 'https://example.com', 'result' => true],
40+
['config' => '*//example.org *//example.com', 'url' => 'https://example.com/a/b/c?test=cat#hello', 'result' => true],
41+
['config' => '*.example.org *.example.com', 'url' => 'https://example.co.uk', 'result' => false],
42+
['config' => ' *.example.org *.example.com ', 'url' => 'https://example.co.uk', 'result' => false],
43+
['config' => '* *.example.com', 'url' => 'https://example.co.uk', 'result' => true],
44+
['config' => '*//example.org *//example.com *//example.co.uk', 'url' => 'https://example.co.uk', 'result' => true],
45+
['config' => '*//example.org *//example.com *//example.co.uk', 'url' => 'https://example.net', 'result' => false],
46+
];
47+
48+
foreach ($testMap as $test) {
49+
$result = (new SsrUrlValidator($test['config']))->allowed($test['url']);
50+
$this->assertEquals($test['result'], $result, "Failed asserting url '{$test['url']}' with config '{$test['config']}' results " . ($test['result'] ? 'true' : 'false'));
51+
}
52+
}
53+
54+
public function test_enssure_allowed()
55+
{
56+
$result = (new SsrUrlValidator('https://example.com'))->ensureAllowed('https://example.com');
57+
$this->assertNull($result);
58+
59+
$this->expectException(HttpFetchException::class);
60+
(new SsrUrlValidator('https://example.com'))->ensureAllowed('https://test.example.com');
61+
}
62+
}

0 commit comments

Comments
 (0)