Skip to content

Commit

Permalink
Security: Added new SSR allow list and validator
Browse files Browse the repository at this point in the history
Included unit tests to cover validator functionality.
Added to webhooks.
Still need to do testing specifically for webhooks.
  • Loading branch information
ssddanbrown committed Aug 26, 2023
1 parent 9100a82 commit c324ad9
Show file tree
Hide file tree
Showing 5 changed files with 137 additions and 0 deletions.
3 changes: 3 additions & 0 deletions app/Activity/DispatchWebhookJob.php
Expand Up @@ -8,6 +8,7 @@
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;
Expand Down Expand Up @@ -53,6 +54,8 @@ public function handle()
$lastError = null;

try {
(new SsrUrlValidator())->ensureAllowed($this->webhook->endpoint);

$response = Http::asJson()
->withOptions(['allow_redirects' => ['strict' => true]])
->timeout($this->webhook->timeout)
Expand Down
9 changes: 9 additions & 0 deletions app/Config/app.php
Expand Up @@ -66,6 +66,15 @@
// 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),
Expand Down
64 changes: 64 additions & 0 deletions 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));
}
}
2 changes: 2 additions & 0 deletions lang/en/errors.php
Expand Up @@ -111,4 +111,6 @@
// 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',
];
59 changes: 59 additions & 0 deletions 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');
}
}

0 comments on commit c324ad9

Please sign in to comment.