New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stored Cross-Site Scripting #575

Closed
ProDigySML opened this Issue Oct 26, 2017 · 10 comments

Comments

Projects
None yet
5 participants
@ProDigySML

ProDigySML commented Oct 26, 2017

For Bug Reports

  • BookStack Version (Found in settings, Please don't put 'latest'): BookStack v0.18.4
  • PHP Version: 7.0
  • MySQL Version: Ver 14.14 Distrib 5.7.20, for Linux (x86_64)
Expected Behavior

Filter out JS code. Any author can write Cross-site scripting payloads and cause issues for the users/

Current Behavior

JS code is not filtered within the page creation

Steps to Reproduce
  1. Create a book
  2. Create a page
  3. While editing the page, choose to edit the source code and add <script>alert(1)</script>
  4. Visit the page (alert box should pop up)
@ibnbay00

This comment has been minimized.

Show comment
Hide comment
@ibnbay00

ibnbay00 Nov 1, 2017

Doesn't work on me... it's fine while I produce ur steps.

ibnbay00 commented Nov 1, 2017

Doesn't work on me... it's fine while I produce ur steps.

@ssddanbrown

This comment has been minimized.

Show comment
Hide comment
@ssddanbrown

ssddanbrown Nov 6, 2017

Member

Hi @ProDigySML, Thanks for opening this issue. Please could you confirm the editor you're using? Markdown or WYSIWYG?

Member

ssddanbrown commented Nov 6, 2017

Hi @ProDigySML, Thanks for opening this issue. Please could you confirm the editor you're using? Markdown or WYSIWYG?

@ProDigySML

This comment has been minimized.

Show comment
Hide comment
@ProDigySML

ProDigySML Nov 21, 2017

@ssddanbrown The editor shouldn't really matter much to be honest (I think I was using the WYSIWYG one though). There should be some server side sanitisation in place, ensuring such bugs don't occur. Maybe something like HTML encoding the user's input may be a good way to fix it :)

ProDigySML commented Nov 21, 2017

@ssddanbrown The editor shouldn't really matter much to be honest (I think I was using the WYSIWYG one though). There should be some server side sanitisation in place, ensuring such bugs don't occur. Maybe something like HTML encoding the user's input may be a good way to fix it :)

@ssddanbrown

This comment has been minimized.

Show comment
Hide comment
@ssddanbrown

ssddanbrown Feb 20, 2018

Member

Huh, Looks like someone filed CVE for this:
https://www.cvedetails.com/cve/CVE-2017-1000462/

Member

ssddanbrown commented Feb 20, 2018

Huh, Looks like someone filed CVE for this:
https://www.cvedetails.com/cve/CVE-2017-1000462/

@sorvani

This comment has been minimized.

Show comment
Hide comment
@sorvani

sorvani Feb 21, 2018

I cannot replicate this with Firefox 58.0.1 and Bookstack v0.20 when using the WYSIWYG editor.

sorvani commented Feb 21, 2018

I cannot replicate this with Firefox 58.0.1 and Bookstack v0.20 when using the WYSIWYG editor.

@sorvani

This comment has been minimized.

Show comment
Hide comment
@sorvani

sorvani Feb 21, 2018

I even tried putting it in the header. and putting it in with the "Source Code" button.
selection_999 659
selection_999 658
selection_999 657
selection_999 656
selection_999 655
selection_999 654
selection_999 653

sorvani commented Feb 21, 2018

I even tried putting it in the header. and putting it in with the "Source Code" button.
selection_999 659
selection_999 658
selection_999 657
selection_999 656
selection_999 655
selection_999 654
selection_999 653

@lommes

This comment has been minimized.

Show comment
Hide comment
@lommes

lommes Feb 21, 2018

Contributor

Ok, let's analyze this a little closer ... I tried to reproduce this in version 0.18.0 without success, but it seems that the sanitation is done by javascript in the editor (tiny mce 4.6.2 in BookStack 0.18.0) because the script tags get remove immediately after closing the sourcecode panel. After manually manipulating the editors content before saving does remove the script-tags too, atleast they are not in the page when i edit it again.

Since this is a javascript based sanitation this might fail depending on browser and version, which, in my opinion, is not related to BookStack.

Contributor

lommes commented Feb 21, 2018

Ok, let's analyze this a little closer ... I tried to reproduce this in version 0.18.0 without success, but it seems that the sanitation is done by javascript in the editor (tiny mce 4.6.2 in BookStack 0.18.0) because the script tags get remove immediately after closing the sourcecode panel. After manually manipulating the editors content before saving does remove the script-tags too, atleast they are not in the page when i edit it again.

Since this is a javascript based sanitation this might fail depending on browser and version, which, in my opinion, is not related to BookStack.

@ssddanbrown

This comment has been minimized.

Show comment
Hide comment
@ssddanbrown

ssddanbrown Mar 5, 2018

Member

Thanks everyone for the research above.
It's likely this was raised with the Markdown editor in use since JavaScript is not escaped in that.
Whether this is an issue or not really depends on how someone is using BookStack. For my own uses I quite like the idea of being able to add a little JS if required but I understand it's not ideal for non-trusted environments.

I know the easy answer here is 'Make it an option' but I like to explore all ideas and opinions otherwise 'Make it an option' is always the answer.

Member

ssddanbrown commented Mar 5, 2018

Thanks everyone for the research above.
It's likely this was raised with the Markdown editor in use since JavaScript is not escaped in that.
Whether this is an issue or not really depends on how someone is using BookStack. For my own uses I quite like the idea of being able to add a little JS if required but I understand it's not ideal for non-trusted environments.

I know the easy answer here is 'Make it an option' but I like to explore all ideas and opinions otherwise 'Make it an option' is always the answer.

@ssddanbrown

This comment has been minimized.

Show comment
Hide comment
@ssddanbrown

ssddanbrown Mar 12, 2018

Member

Marked for next release. Plan to have an .env var to toggle <script> tag escaping on page render. Will default to on (Escaped) by default. Will run on render instead of on save to allow option toggling at any point.

Member

ssddanbrown commented Mar 12, 2018

Marked for next release. Plan to have an .env var to toggle <script> tag escaping on page render. Will default to on (Escaped) by default. Will run on render instead of on save to allow option toggling at any point.

@ssddanbrown ssddanbrown self-assigned this Mar 12, 2018

@ssddanbrown

This comment has been minimized.

Show comment
Hide comment
@ssddanbrown

ssddanbrown Mar 17, 2018

Member

For reference, Setting ALLOW_CONTENT_SCRIPTS=true in the .env will prevent escaping.

Member

ssddanbrown commented Mar 17, 2018

For reference, Setting ALLOW_CONTENT_SCRIPTS=true in the .env will prevent escaping.

lassebenni added a commit to lassebenni/BookStack that referenced this issue Apr 13, 2018

Release (#1)
* Started draw.io integration

* Updated pull-request info to be clearer

Also updated dev version

* Adds code to allow deletion of users via cmd line.

Fixes BookStackApp#579

Command:

php artisan bookstack:delete-users

Signed-off-by: Abijeet <abijeetpatro@gmail.com>

* Adds button to allow users to toggle the book view via the books list page.

Closes BookStackApp#613

Signed-off-by: Abijeet <abijeetpatro@gmail.com>

* Adds test cases and fixes an issue with the permission checking.

Signed-off-by: Abijeet <abijeetpatro@gmail.com>

* Enabled session in 404 responses

Fixes #634

* Adds font-size to ol to ensure that they are uniform.

Fixes BookStackApp#643

Signed-off-by: Abijeet <abijeetpatro@gmail.com>

* Standardised admin role check

* Updated book view change to PATCH + other amends

Moved toggle to right of header bar and added unique text and icon for
each view type.

Removed old profile setting to keep things clean.

* Enabled system-storage of drawings made via draw.io

* Fixed broken table/ol/ul page includes

Fixes BookStackApp#640

* Fixed failing book view test

Also ensured setting system localcache is cleared correctly

* Checks the target and the source book before performing the sort.

Signed-off-by: Abijeet <abijeetpatro@gmail.com>

* Changed the sort view to only show books to which we have an update permission.

Signed-off-by: Abijeet <abijeetpatro@gmail.com>

* Adds test case for sorting permissions.

Signed-off-by: Abijeet <abijeetpatro@gmail.com>

* Added view override support

Relates to BookStackApp#652

* Refactored the code to first check for the permissions before sorting the book.

Signed-off-by: Abijeet <abijeetpatro@gmail.com>

* Adds overflow:auto to popup content to allow it to scroll in lower res.

Fixes BookStackApp#650

Signed-off-by: Abijeet <abijeetpatro@gmail.com>

* Added ability to secure images behind auth

Still in testing.
Adds STORAGE_TYPE=local_secure option for setting images to be behind
auth. Stores images alongside attachments in /storage/uploads/images.

* Refactored book sort using collections

* Added drawing update ability

* Added drawing icon and made drawio disablable

* Extracted draw.io functionality to own file

* Added drawio abilities to markdown editor

* Added Swedish translation

* Added Swedish locale to config

* Added drawing endpoint tests

Also refactored ImageTests away from BrowserKit
Also added image upload type validation.

* Fixed test failing from missing baseURL

Also updated image upload test to delete before upload to prevent failed
tests breaking subsequent tests.

* Actually fixed the BaseURL this time 🤦

* #630: Deleting user's profile pics on deleting of user account (BookStackApp#646)

* Issue-630: Fixed issue with deleting user profile pics when deleting a user.

* Issue #630: Deleting user's profile pics on deleting of user account

* Issue-630: Added test case for deleting user

* Updated user profile image delete to delete all uploads

Also moved test and made more comprehensive

* Adapted code insert area and language select for mobile

* Set /app PHP code to PSR-2 standard

Also adde draw.io to attribution list.

Closes BookStackApp#649

* Fixed validation issue on register post

Added test to cover and also cleaned up RegisterController comments.

Fixes #670

* Added command to add a new admin user

Closes BookStackApp#609

* Set markdown editor preview width to 100%

Fixes BookStackApp#658

* Add twitch socialite auth provider

* Update...

* add support for gitlab authentification

* add missing lock file

* Corrected the keys for okta auth

* Update services.php

* Update .env.example

* reduced icon size

* add missing icon, fix name conventions

* Update all.blade.php

* Made default books view configurable in .env

Under 'APP_VIEWS_BOOKS' key.
Closes BookStackApp#675

* Added simplified Chinese(zh-CN) language

* Added 'zh_CN' to app.locales

* Prevent image manager search from reloading page

Fixes BookStackApp#697

* Fixed code block wrapping on export

Now wraps instead of running off the page.

Fixed BookStackApp#676

* Updated grid view to use CSS grid and flexbox

Provides a cleaner height-matched design.
Closes BookStackApp#701

* Updated twitch SVG icon with vector SVG

* Fixed error when accessing non-authed attachment

Also updated attachment tests to use standard test-case.
Fixes BookStackApp#681

* Fixed text overflow in various views

Fixes BookStackApp#669

* Tweaked some comments

* Updated assets for release v0.20.0

* Made it possible to override icons via custom theme

* Started migration to SVG icons

* Finished migrated from icon-font to SVG

* Updated 'Spanish Argentina' translation.

* Added ability to configure email sender name

Added env variable MAIL_FROM_NAME to allow the email sender name to be
customised. Also added MAIL_FROM to .env.example

* Updated 'Spanish Argentina' translation.

* Add CACHE_PREFIX to the .env.example file

We had some problems with multiple BookStack instances using the same caching server. Perhaps it's a good idea to have this available in the `.env.example` file.

* fix markdown editor resizing with long strings

* typo in readme.md

* add missing polish translations for comments

* Update .env.example

* Attempted move to webpack again

* Reorganised dev-deps and updated moment

* Update Italian translation

* Fixes issue with the validation message not being translated.

Signed-off-by: Abijeet <abijeetpatro@gmail.com>

* Finished off intitial conversion to webpack

* Moved jQuery to use NPM and fixed some asset refs

* Added togglable script escaping to page content

Configurable via 'ALLOW_CONTENT_SCRIPTS' env variable.
Fixes BookStackApp#575

* Updated exports to use DejaVu font

Provides potentially better language font coverage.

* Markdown editor image paste sets cursor correctly

Now sets cursor to alt text rather than end of placeholder image.
Fixed BookStackApp#751

* Prevented markdown editor pushing out toolbar

* Fixed up notification styling a little

* Fixed large content previews and improved mobile styles

Listing content previews no longer pre-wrap and instead simply break
correctly.
Updated titles to ensure they break on mobile devices.
Reduced spacing and font sizes on mobile to better adjust content to
screen size.

Fixes BookStackApp#739

* Fixes a number of issues with the image uploader. Read below,

- Added a remove link to remove files that have an error.
- Error will appear below the progress bar.
- Hovering on dz-image or dz-details will display the error message. Otherwise error message was covering the remove link as well.
- Removed styling around the file size.
- Removed gradient effect in accordance with BookStack styling.
- Dropzone filenae will not overflow the container element. Also done for page attachments
- Added a 'uploaded' error message. this error was being thrown when the file size exceeded the server configured file size. (https://stackoverflow.com/a/42934387/903324)

Towards BookStackApp#741

Signed-off-by: Abijeet <abijeetpatro@gmail.com>

* Update activities.php

* Update auth.php

* Update common.php

* Update components.php

* Update settings.php

* Fixes the icons not being aligned properly in attached items section for the page.

Also formatting.

Signed-off-by: Abijeet <abijeetpatro@gmail.com>

* Update entities.php

* Updated webpack SCSS extract to provide sourcemaps

* Cleaned some form styling

Removed uppercasing of labels to make a little friendlier.
Extracted out toggleswitch JS into own component.
Improved basic code input for html-head-input area.

* Use autodiscover for dev packages

Allows installation with `composer install --no-dev`
Fixed BookStackApp#742

* Updated outline button styles for svg icons

* Removing the selected image and clearing the dropdzone on dialog close.

Towards BookStackApp#741

Signed-off-by: Abijeet <abijeetpatro@gmail.com>

* Not resizing gif images.

See - Intervention/image#176

Fixes BookStackApp#223

Signed-off-by: Abijeet <abijeetpatro@gmail.com>

* Fixes an issue with handling of large image files.

Signed-off-by: Abijeet <abijeetpatro@gmail.com>

* update japanese translation

* fix entities.php

* Fixed failing tests

Fixed syntax error in french translations.
Removed 'required' on image validation which was breaking tests

* Made search more efficient and tweaked weighting

Added per-entity weighting changes.
Now Books score higher than chapters which score higher than pages.

Reduced queries required on search by only searching once but at a
higher count to see if there's another page.

* Fixed incorrect search logic in last commit

Incorrect cross-entity pagination could lead to hidden entities.

* Properly escaped search results

Prevents vue-like syntax in results causing errors.
Related to BookStackApp#748

* Updated create routes to prevent slug clashes

Fixes BookStackApp#758

* Removed invalid bracket from view

* Ensured uploaded system images remain public

Also added tests to cover local_secure image storage.

Fixes BookStackApp#725

* Updated icons with height

Fixes issues within IE

* Updated assets for release v0.20.1

* Fixed export style paths

* Updated the version because i'm such a plonker

And forgot to do this last release.
I wonder if there's a simple commit hook that could prevent the same two
versions twice in a row?

lassebenni added a commit to lassebenni/BookStack that referenced this issue Apr 13, 2018

Release merge try #2 (#2)
* Started draw.io integration

* Updated pull-request info to be clearer

Also updated dev version

* Adds code to allow deletion of users via cmd line.

Fixes BookStackApp#579

Command:

php artisan bookstack:delete-users

Signed-off-by: Abijeet <abijeetpatro@gmail.com>

* Adds button to allow users to toggle the book view via the books list page.

Closes BookStackApp#613

Signed-off-by: Abijeet <abijeetpatro@gmail.com>

* Adds test cases and fixes an issue with the permission checking.

Signed-off-by: Abijeet <abijeetpatro@gmail.com>

* Enabled session in 404 responses

Fixes #634

* Adds font-size to ol to ensure that they are uniform.

Fixes BookStackApp#643

Signed-off-by: Abijeet <abijeetpatro@gmail.com>

* Standardised admin role check

* Updated book view change to PATCH + other amends

Moved toggle to right of header bar and added unique text and icon for
each view type.

Removed old profile setting to keep things clean.

* Enabled system-storage of drawings made via draw.io

* Fixed broken table/ol/ul page includes

Fixes BookStackApp#640

* Fixed failing book view test

Also ensured setting system localcache is cleared correctly

* Checks the target and the source book before performing the sort.

Signed-off-by: Abijeet <abijeetpatro@gmail.com>

* Changed the sort view to only show books to which we have an update permission.

Signed-off-by: Abijeet <abijeetpatro@gmail.com>

* Adds test case for sorting permissions.

Signed-off-by: Abijeet <abijeetpatro@gmail.com>

* Added view override support

Relates to BookStackApp#652

* Refactored the code to first check for the permissions before sorting the book.

Signed-off-by: Abijeet <abijeetpatro@gmail.com>

* Adds overflow:auto to popup content to allow it to scroll in lower res.

Fixes BookStackApp#650

Signed-off-by: Abijeet <abijeetpatro@gmail.com>

* Added ability to secure images behind auth

Still in testing.
Adds STORAGE_TYPE=local_secure option for setting images to be behind
auth. Stores images alongside attachments in /storage/uploads/images.

* Refactored book sort using collections

* Added drawing update ability

* Added drawing icon and made drawio disablable

* Extracted draw.io functionality to own file

* Added drawio abilities to markdown editor

* Added Swedish translation

* Added Swedish locale to config

* Added drawing endpoint tests

Also refactored ImageTests away from BrowserKit
Also added image upload type validation.

* Fixed test failing from missing baseURL

Also updated image upload test to delete before upload to prevent failed
tests breaking subsequent tests.

* Actually fixed the BaseURL this time 🤦

* #630: Deleting user's profile pics on deleting of user account (BookStackApp#646)

* Issue-630: Fixed issue with deleting user profile pics when deleting a user.

* Issue #630: Deleting user's profile pics on deleting of user account

* Issue-630: Added test case for deleting user

* Updated user profile image delete to delete all uploads

Also moved test and made more comprehensive

* Adapted code insert area and language select for mobile

* Set /app PHP code to PSR-2 standard

Also adde draw.io to attribution list.

Closes BookStackApp#649

* Fixed validation issue on register post

Added test to cover and also cleaned up RegisterController comments.

Fixes #670

* Added command to add a new admin user

Closes BookStackApp#609

* Set markdown editor preview width to 100%

Fixes BookStackApp#658

* Add twitch socialite auth provider

* Update...

* add support for gitlab authentification

* add missing lock file

* Corrected the keys for okta auth

* Update services.php

* Update .env.example

* reduced icon size

* add missing icon, fix name conventions

* Update all.blade.php

* Made default books view configurable in .env

Under 'APP_VIEWS_BOOKS' key.
Closes BookStackApp#675

* Added simplified Chinese(zh-CN) language

* Added 'zh_CN' to app.locales

* Prevent image manager search from reloading page

Fixes BookStackApp#697

* Fixed code block wrapping on export

Now wraps instead of running off the page.

Fixed BookStackApp#676

* Updated grid view to use CSS grid and flexbox

Provides a cleaner height-matched design.
Closes BookStackApp#701

* Updated twitch SVG icon with vector SVG

* Fixed error when accessing non-authed attachment

Also updated attachment tests to use standard test-case.
Fixes BookStackApp#681

* Fixed text overflow in various views

Fixes BookStackApp#669

* Tweaked some comments

* Updated assets for release v0.20.0

* Made it possible to override icons via custom theme

* Started migration to SVG icons

* Finished migrated from icon-font to SVG

* Updated 'Spanish Argentina' translation.

* Added ability to configure email sender name

Added env variable MAIL_FROM_NAME to allow the email sender name to be
customised. Also added MAIL_FROM to .env.example

* Updated 'Spanish Argentina' translation.

* Add CACHE_PREFIX to the .env.example file

We had some problems with multiple BookStack instances using the same caching server. Perhaps it's a good idea to have this available in the `.env.example` file.

* fix markdown editor resizing with long strings

* typo in readme.md

* add missing polish translations for comments

* Update .env.example

* Attempted move to webpack again

* Reorganised dev-deps and updated moment

* Update Italian translation

* Fixes issue with the validation message not being translated.

Signed-off-by: Abijeet <abijeetpatro@gmail.com>

* Finished off intitial conversion to webpack

* Moved jQuery to use NPM and fixed some asset refs

* Added togglable script escaping to page content

Configurable via 'ALLOW_CONTENT_SCRIPTS' env variable.
Fixes BookStackApp#575

* Updated exports to use DejaVu font

Provides potentially better language font coverage.

* Markdown editor image paste sets cursor correctly

Now sets cursor to alt text rather than end of placeholder image.
Fixed BookStackApp#751

* Prevented markdown editor pushing out toolbar

* Fixed up notification styling a little

* Fixed large content previews and improved mobile styles

Listing content previews no longer pre-wrap and instead simply break
correctly.
Updated titles to ensure they break on mobile devices.
Reduced spacing and font sizes on mobile to better adjust content to
screen size.

Fixes BookStackApp#739

* Fixes a number of issues with the image uploader. Read below,

- Added a remove link to remove files that have an error.
- Error will appear below the progress bar.
- Hovering on dz-image or dz-details will display the error message. Otherwise error message was covering the remove link as well.
- Removed styling around the file size.
- Removed gradient effect in accordance with BookStack styling.
- Dropzone filenae will not overflow the container element. Also done for page attachments
- Added a 'uploaded' error message. this error was being thrown when the file size exceeded the server configured file size. (https://stackoverflow.com/a/42934387/903324)

Towards BookStackApp#741

Signed-off-by: Abijeet <abijeetpatro@gmail.com>

* Update activities.php

* Update auth.php

* Update common.php

* Update components.php

* Update settings.php

* Fixes the icons not being aligned properly in attached items section for the page.

Also formatting.

Signed-off-by: Abijeet <abijeetpatro@gmail.com>

* Update entities.php

* Updated webpack SCSS extract to provide sourcemaps

* Cleaned some form styling

Removed uppercasing of labels to make a little friendlier.
Extracted out toggleswitch JS into own component.
Improved basic code input for html-head-input area.

* Use autodiscover for dev packages

Allows installation with `composer install --no-dev`
Fixed BookStackApp#742

* Updated outline button styles for svg icons

* Removing the selected image and clearing the dropdzone on dialog close.

Towards BookStackApp#741

Signed-off-by: Abijeet <abijeetpatro@gmail.com>

* Not resizing gif images.

See - Intervention/image#176

Fixes BookStackApp#223

Signed-off-by: Abijeet <abijeetpatro@gmail.com>

* Fixes an issue with handling of large image files.

Signed-off-by: Abijeet <abijeetpatro@gmail.com>

* update japanese translation

* fix entities.php

* Fixed failing tests

Fixed syntax error in french translations.
Removed 'required' on image validation which was breaking tests

* Made search more efficient and tweaked weighting

Added per-entity weighting changes.
Now Books score higher than chapters which score higher than pages.

Reduced queries required on search by only searching once but at a
higher count to see if there's another page.

* Fixed incorrect search logic in last commit

Incorrect cross-entity pagination could lead to hidden entities.

* Properly escaped search results

Prevents vue-like syntax in results causing errors.
Related to BookStackApp#748

* Updated create routes to prevent slug clashes

Fixes BookStackApp#758

* Removed invalid bracket from view

* Ensured uploaded system images remain public

Also added tests to cover local_secure image storage.

Fixes BookStackApp#725

* Updated icons with height

Fixes issues within IE

* Updated assets for release v0.20.1

* Fixed export style paths

* Updated the version because i'm such a plonker

And forgot to do this last release.
I wonder if there's a simple commit hook that could prevent the same two
versions twice in a row?
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment