github-mirror/BookStack
2
0
Fork 0
mirror of https://github.com/BookStackApp/BookStack.git synced 2024-11-26 10:13:38 +08:00
Code Issues Actions 6 Packages Projects Releases Wiki Activity

Merge branch '3636-security-patch' into development

Browse Source
This commit is contained in:
Dan Brown 2022-08-11 15:15:19 +01:00
commit d6235bcf92
No known key found for this signature in database
GPG Key ID: 46D9F943C24A2EF9
5 changed files with 34 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
app/Http/Controllers/Api/PageApiController.php
View File

@ -86,6 +86,9 @@ class PageApiController extends ApiController
*
* Pages will always have HTML content. They may have markdown content
* if the markdown editor was used to last update the page.
*
* See the "Content Security" section of these docs for security considerations when using
* the page content returned from this endpoint.
*/
public function read(string $id)
{

5
app/Util/HtmlContentFilter.php
View File

@ -45,6 +45,11 @@ class HtmlContentFilter
$badIframes = $xPath->query('//*[' . static::xpathContains('@src', 'data:') . '] | //*[' . static::xpathContains('@src', 'javascript:') . '] | //*[@srcdoc]');
static::removeNodes($badIframes);
// Remove tags hiding JavaScript or data uris in values attribute.
// For example, SVG animate tag can exploit javascript in values.
$badValuesTags = $xPath->query('//*[' . static::xpathContains('@values', 'data:') . '] | //*[' . static::xpathContains('@values', 'javascript:') . ']');
static::removeNodes($badValuesTags);
// Remove elements with a xlink:href attribute
// Used in SVG but deprecated anyway, so we'll be a bit more heavy-handed here.
$xlinkHrefAttributes = $xPath->query('//@*[contains(name(), \'xlink:href\')]');

1
resources/views/api-docs/index.blade.php
View File

@ -16,6 +16,7 @@
<div class="mb-xs"><a href="#listing-endpoints">Listing Endpoints</a></div>
<div class="mb-xs"><a href="#error-handling">Error Handling</a></div>
<div class="mb-xs"><a href="#rate-limits">Rate Limits</a></div>
<div class="mb-xs"><a href="#content-security">Content Security</a></div>
</div>
@foreach($docs as $model => $endpoints)

16
resources/views/api-docs/parts/getting-started.blade.php
View File

@ -179,4 +179,20 @@ API_REQUESTS_PER_MIN=180</code></pre>
It's generally good practice to limit requests made from your API client, where possible, to avoid
affecting normal use of the system caused by over-consuming system resources.
Keep in mind there may be other rate-limiting factors such as web-server & firewall controls.
</p>
<hr>
<h5 id="content-security" class="text-mono mb-m">Content Security</h5>
<p>
Many of the available endpoints will return content that has been provided by user input.
Some of this content may be provided in a certain data-format (Such as HTML or Markdown for page content).
Such content is not guaranteed to be safe so keep security in mind when dealing with such user-input.
In some cases, the system will apply some filtering to content in an attempt to prevent certain vulnerabilities, but
this is not assured to be a bullet-proof defence.
</p>
<p>
Within its own interfaces, unless disabled, the system makes use of Content Security Policy (CSP) rules to heavily negate
cross-site scripting vulnerabilities from user content. If displaying user content externally, it's advised you
also use defences such as CSP or the disabling of JavaScript completely.
</p>

13
tests/Entity/PageContentTest.php
View File

@ -325,11 +325,14 @@ class PageContentTest extends TestCase
$pageView->assertDontSee('abc123abc123');
}
public function test_svg_xlink_hrefs_are_removed()
public function test_svg_script_usage_is_removed()
{
$checks = [
'<svg id="test" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" width="100" height="100"><a xlink:href="javascript:alert(document.domain)"><rect x="0" y="0" width="100" height="100" /></a></svg>',
'<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink"><use xlink:href="data:application/xml;base64 ,PHN2ZyB4bWxucz0iaHR0cDovL3d3dy53My5vcmcvMjAwMC9zdmciIHhtbG5zOnhsaW5rPSJodHRwOi8vd3d3LnczLm9yZy8xOTk5L3hsaW5rIj4KPGRlZnM+CjxjaXJjbGUgaWQ9InRlc3QiIHI9IjAiIGN4PSIwIiBjeT0iMCIgc3R5bGU9ImZpbGw6ICNGMDAiPgo8c2V0IGF0dHJpYnV0ZU5hbWU9ImZpbGwiIGF0dHJpYnV0ZVR5cGU9IkNTUyIgb25iZWdpbj0nYWxlcnQoZG9jdW1lbnQuZG9tYWluKScKb25lbmQ9J2FsZXJ0KCJvbmVuZCIpJyB0bz0iIzAwRiIgYmVnaW49IjBzIiBkdXI9Ijk5OXMiIC8+CjwvY2lyY2xlPgo8L2RlZnM+Cjx1c2UgeGxpbms6aHJlZj0iI3Rlc3QiLz4KPC9zdmc+#test"/></svg>',
'<svg><animate href=#xss attributeName=href values=javascript:alert(1) /></svg>',
'<svg><animate href="#xss" attributeName="href" values="a;javascript:alert(1)" /></svg>',
'<svg><animate href="#xss" attributeName="href" values="a;data:alert(1)" /></svg>',
];
$this->asEditor();
@ -341,9 +344,11 @@ class PageContentTest extends TestCase
$pageView = $this->get($page->getUrl());
$pageView->assertStatus(200);
$this->withHtml($pageView)->assertElementNotContains('.page-content', 'alert');
$this->withHtml($pageView)->assertElementNotContains('.page-content', 'xlink:href');
$this->withHtml($pageView)->assertElementNotContains('.page-content', 'application/xml');
$html = $this->withHtml($pageView);
$html->assertElementNotContains('.page-content', 'alert');
$html->assertElementNotContains('.page-content', 'xlink:href');
$html->assertElementNotContains('.page-content', 'application/xml');
$html->assertElementNotContains('.page-content', 'javascript');
}
}