From 4de719b3254ae81a7700401832c0a09c4e295525 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Tue, 6 Aug 2019 20:33:51 +0100 Subject: [PATCH 1/3] Prevented potential apache image dir listing Closes #1545 --- public/.htaccess | 2 +- public/uploads/.gitignore | 3 ++- public/uploads/.htaccess | 1 + 3 files changed, 4 insertions(+), 2 deletions(-) create mode 100755 public/uploads/.htaccess diff --git a/public/.htaccess b/public/.htaccess index 8eb2dd0dd..0d55354ec 100644 --- a/public/.htaccess +++ b/public/.htaccess @@ -1,6 +1,6 @@ - Options -MultiViews + Options -MultiViews -Indexes RewriteEngine On diff --git a/public/uploads/.gitignore b/public/uploads/.gitignore index c96a04f00..cb7328e19 100755 --- a/public/uploads/.gitignore +++ b/public/uploads/.gitignore @@ -1,2 +1,3 @@ * -!.gitignore \ No newline at end of file +!.gitignore +!.htaccess \ No newline at end of file diff --git a/public/uploads/.htaccess b/public/uploads/.htaccess new file mode 100755 index 000000000..45552cb63 --- /dev/null +++ b/public/uploads/.htaccess @@ -0,0 +1 @@ +Options -Indexes \ No newline at end of file From 2955f414ddc29f725fd940febf19894255252875 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Tue, 6 Aug 2019 21:08:24 +0100 Subject: [PATCH 2/3] Added iframe JS and data url escaping Related to #1531 --- app/Entities/Repos/EntityRepo.php | 6 ++++++ tests/Entity/PageContentTest.php | 34 +++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/app/Entities/Repos/EntityRepo.php b/app/Entities/Repos/EntityRepo.php index aad9a1205..7ca25b785 100644 --- a/app/Entities/Repos/EntityRepo.php +++ b/app/Entities/Repos/EntityRepo.php @@ -765,6 +765,12 @@ class EntityRepo $scriptElem->parentNode->removeChild($scriptElem); } + // Remove data or JavaScript iFrames + $badIframes = $xPath->query('//*[contains(@src, \'data:\')] | //*[contains(@src, \'javascript:\')]'); + foreach ($badIframes as $badIframe) { + $badIframe->parentNode->removeChild($badIframe); + } + // Remove 'on*' attributes $onAttributes = $xPath->query('//@*[starts-with(name(), \'on\')]'); foreach ($onAttributes as $attr) { diff --git a/tests/Entity/PageContentTest.php b/tests/Entity/PageContentTest.php index c80b5f1d9..b447a7c5d 100644 --- a/tests/Entity/PageContentTest.php +++ b/tests/Entity/PageContentTest.php @@ -80,6 +80,7 @@ class PageContentTest extends TestCase $page->save(); $pageView = $this->get($page->getUrl()); + $pageView->assertStatus(200); $pageView->assertDontSee($script); $pageView->assertSee('abc123abc123'); } @@ -103,12 +104,42 @@ class PageContentTest extends TestCase $page->save(); $pageView = $this->get($page->getUrl()); + $pageView->assertStatus(200); $pageView->assertElementNotContains('.page-content', ''); } } + public function test_iframe_js_and_base64_urls_are_removed() + { + $checks = [ + '', + '', + '', + '', + + ]; + + $this->asEditor(); + $page = Page::first(); + + foreach ($checks as $check) { + $page->html = $check; + $page->save(); + + $pageView = $this->get($page->getUrl()); + $pageView->assertStatus(200); + $pageView->assertElementNotContains('.page-content', ''); + $pageView->assertElementNotContains('.page-content', 'src='); + $pageView->assertElementNotContains('.page-content', 'javascript:'); + $pageView->assertElementNotContains('.page-content', 'data:'); + $pageView->assertElementNotContains('.page-content', 'base64'); + } + + } + public function test_page_inline_on_attributes_removed_by_default() { $this->asEditor(); @@ -118,6 +149,7 @@ class PageContentTest extends TestCase $page->save(); $pageView = $this->get($page->getUrl()); + $pageView->assertStatus(200); $pageView->assertDontSee($script); $pageView->assertSee('

Hello

'); } @@ -130,6 +162,7 @@ class PageContentTest extends TestCase '
Lorem ipsum dolor sit amet.

Hello

', '
Lorem ipsum dolor sit amet.

Hello

', '
Lorem ipsum dolor sit amet.

Hello

', + 'has($name)) class="text-neg" @endif @if(isset($placeholder)) placeholder="{{$placeholder}}" @endif + @if(isset($disabled) && $disabled) disabled="disabled" @endif @if(isset($tabindex)) tabindex="{{$tabindex}}" @endif @if(isset($model) || old($name)) value="{{ old($name) ? old($name) : $model->$name}}" @endif> @if($errors->has($name)) diff --git a/resources/views/users/form.blade.php b/resources/views/users/form.blade.php index 96beb7b2f..3d073b2c8 100644 --- a/resources/views/users/form.blade.php +++ b/resources/views/users/form.blade.php @@ -19,7 +19,7 @@
@if($authMethod !== 'ldap' || userCan('users-manage')) - @include('form.text', ['name' => 'email']) + @include('form.text', ['name' => 'email', 'disabled' => !userCan('users-manage')]) @endif
diff --git a/tests/Permissions/RolesTest.php b/tests/Permissions/RolesTest.php index 5bbdcf0bb..a1f193643 100644 --- a/tests/Permissions/RolesTest.php +++ b/tests/Permissions/RolesTest.php @@ -119,6 +119,43 @@ class RolesTest extends BrowserKitTest $this->actingAs($this->user)->visit('/')->dontSee($usersLink); } + public function test_user_cannot_change_email_unless_they_have_manage_users_permission() + { + $userProfileUrl = '/settings/users/' . $this->user->id; + $originalEmail = $this->user->email; + $this->actingAs($this->user); + + $this->visit($userProfileUrl) + ->assertResponseOk() + ->seeElement('input[name=email][disabled]'); + $this->put($userProfileUrl, [ + 'name' => 'my_new_name', + 'email' => 'new_email@example.com', + ]); + $this->seeInDatabase('users', [ + 'id' => $this->user->id, + 'email' => $originalEmail, + 'name' => 'my_new_name', + ]); + + $this->giveUserPermissions($this->user, ['users-manage']); + + $this->visit($userProfileUrl) + ->assertResponseOk() + ->dontSeeElement('input[name=email][disabled]') + ->seeElement('input[name=email]'); + $this->put($userProfileUrl, [ + 'name' => 'my_new_name_2', + 'email' => 'new_email@example.com', + ]); + + $this->seeInDatabase('users', [ + 'id' => $this->user->id, + 'email' => 'new_email@example.com', + 'name' => 'my_new_name_2', + ]); + } + public function test_user_roles_manage_permission() { $this->actingAs($this->user)->visit('/settings/roles')