Skip to content

Commit 06901b8

Browse files
committed
Comments: Added HTML filter on load, tinymce elem filtering
- Added filter on load to help prevent potentially dangerous comment HTML in DB at load time (if it gets passed input filtering, or is existing). - Added TinyMCE valid_elements for input wysiwygs, to gracefully degrade content at point of user-view, rather than surprising the user by stripping content, which TinyMCE would show, post-save.
1 parent e9a19d5 commit 06901b8

File tree

4 files changed

+27
-4
lines changed

4 files changed

+27
-4
lines changed

app/Activity/Models/Comment.php

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
use BookStack\App\Model;
66
use BookStack\Users\Models\HasCreatorAndUpdater;
7+
use BookStack\Util\HtmlContentFilter;
78
use Illuminate\Database\Eloquent\Factories\HasFactory;
89
use Illuminate\Database\Eloquent\Relations\BelongsTo;
910
use Illuminate\Database\Eloquent\Relations\MorphTo;
@@ -73,4 +74,9 @@ public function logDescriptor(): string
7374
{
7475
return "Comment #{$this->local_id} (ID: {$this->id}) for {$this->entity_type} (ID: {$this->entity_id})";
7576
}
77+
78+
public function safeHtml(): string
79+
{
80+
return HtmlContentFilter::removeScriptsFromHtmlString($this->html ?? '');
81+
}
7682
}

resources/js/wysiwyg/config.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -339,6 +339,7 @@ export function buildForInput(options) {
339339
toolbar: 'bold italic link bullist numlist',
340340
content_style: getContentStyle(options),
341341
file_picker_types: 'file',
342+
valid_elements: 'p,a[href|title],ol,ul,li,strong,em,br',
342343
file_picker_callback: filePickerCallback,
343344
init_instance_callback(editor) {
344345
addCustomHeadContent(editor.getDoc());

resources/views/comments/comment.blade.php

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
1+
@php
2+
$commentHtml = $comment->safeHtml();
3+
@endphp
14
<div component="{{ $readOnly ? '' : 'page-comment' }}"
25
option:page-comment:comment-id="{{ $comment->id }}"
36
option:page-comment:comment-local-id="{{ $comment->local_id }}"
@@ -71,13 +74,13 @@ class="comment-box">
7174
<a class="text-muted text-small" href="#comment{{ $comment->parent_id }}">@icon('reply'){{ trans('entities.comment_in_reply_to', ['commentId' => '#' . $comment->parent_id]) }}</a>
7275
</p>
7376
@endif
74-
{!! $comment->html !!}
77+
{!! $commentHtml !!}
7578
</div>
7679

7780
@if(!$readOnly && userCan('comment-update', $comment))
7881
<form novalidate refs="page-comment@form" hidden class="content pt-s px-s block">
7982
<div class="form-group description-input">
80-
<textarea refs="page-comment@input" name="html" rows="3" placeholder="{{ trans('entities.comment_placeholder') }}">{{ $comment->html }}</textarea>
83+
<textarea refs="page-comment@input" name="html" rows="3" placeholder="{{ trans('entities.comment_placeholder') }}">{{ $commentHtml }}</textarea>
8184
</div>
8285
<div class="form-group text-right">
8386
<button type="button" class="button outline" refs="page-comment@form-cancel">{{ trans('common.cancel') }}</button>

tests/Entity/CommentTest.php

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -82,11 +82,10 @@ public function test_comment_delete()
8282

8383
public function test_scripts_cannot_be_injected_via_comment_html()
8484
{
85-
$this->asAdmin();
8685
$page = $this->entities->page();
8786

8887
$script = '<script>const a = "script";</script><p onclick="1">My lovely comment</p>';
89-
$this->postJson("/comment/$page->id", [
88+
$this->asAdmin()->postJson("/comment/$page->id", [
9089
'html' => $script,
9190
]);
9291

@@ -104,6 +103,20 @@ public function test_scripts_cannot_be_injected_via_comment_html()
104103
$pageView->assertSee('<p>My lovely comment</p><p>updated</p>');
105104
}
106105

106+
public function test_scripts_are_removed_even_if_already_in_db()
107+
{
108+
$page = $this->entities->page();
109+
Comment::factory()->create([
110+
'html' => '<script>superbadscript</script><p onclick="superbadonclick">scriptincommentest</p>',
111+
'entity_type' => 'page', 'entity_id' => $page
112+
]);
113+
114+
$resp = $this->asAdmin()->get($page->getUrl());
115+
$resp->assertSee('scriptincommentest', false);
116+
$resp->assertDontSee('superbadscript', false);
117+
$resp->assertDontSee('superbadonclick', false);
118+
}
119+
107120
public function test_reply_comments_are_nested()
108121
{
109122
$this->asAdmin();

0 commit comments

Comments
 (0)