Skip to content
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 (XSS) #228

Closed
l4rm4nd opened this issue Nov 27, 2020 · 11 comments
Closed

Stored Cross-Site Scripting (XSS) #228

l4rm4nd opened this issue Nov 27, 2020 · 11 comments
Assignees
Labels
bug Something isn't working important

Comments

@l4rm4nd
Copy link

l4rm4nd commented Nov 27, 2020

Description
Improper validation of user input leads to stored cross-site scripting (XSS) or HTML injection in the papermerge web application. If a user inserts JavaScript or HTML code into a folder name, the specified payload will be executed on opening the folder.

Expected
Specifying potentially malicious client side code should not be executed in the web application by the browser.

Actual
The browser successfully executes the specified JS or HTML payloads if the newly created folder is opened.

Steps to reproduce

  1. Login to papermerge web application https://demo.papermerge.com/admin/browse
  2. Create a new folder named "XSS Folder<script>alert('XSS');</script>" without the quotes
  3. Open the newly created folder with XSS payload and experience a JavaScript XSS popup saying "XSS".

Impact
This may allow an attacker to steal sensitive session information or CSRF tokens for executing a Cross-Site Request Forgery attack.

Likelihood
Authentication is required to access the papermerge web application.

Recommendation
Do not trust any user input and validate inputs properly. See https://owasp.org/www-community/attacks/xss/

Info:
Tested in the publicly available demo page. https://demo.papermerge.com/admin/browse

@l4rm4nd l4rm4nd added the bug Something isn't working label Nov 27, 2020
@l4rm4nd
Copy link
Author

l4rm4nd commented Nov 27, 2020

Just a quick proof of concept:

grafik

@ciur
Copy link
Owner

ciur commented Nov 28, 2020

@l4rm4nd, really good one! Thank you for opening this security issue. I will fix it and ship it as part of 1.5.1 release.
Thank you!

@ciur ciur added this to the Version 1.5.1 milestone Nov 28, 2020
@ciur ciur added the important label Nov 28, 2020
@l4rm4nd
Copy link
Author

l4rm4nd commented Nov 28, 2020

BTW, this also works for uploaded file names.

Steps to reproduce

  1. Login to papermerge web application https://demo.papermerge.com/admin/browse
  2. Upload a new file with the name "><svg onload=alert(1)>.png
  3. Experience a JavaScript XSS popup saying "1".

Proof of concept
XSS PNG file uploaded in a "Test" folder of the demo papermerge webpage.
https://demo.papermerge.com/admin/browse#19

image

@l4rm4nd
Copy link
Author

l4rm4nd commented Nov 28, 2020

Payloads are also reflected in the logs area https://demo.papermerge.com/admin/logs
image

@ciur
Copy link
Owner

ciur commented Nov 28, 2020

@l4rm4nd, oh, man, thank you for your security audit. I will fix issues in following days and release 1.5.1 with those security fixes.
Thanks again!

@l4rm4nd
Copy link
Author

l4rm4nd commented Nov 28, 2020

Sure, no worries! Thanks for your fast replies and the will to fix the issues.

BTW, tags are also susceptible to XSS.

Steps to reproduce

  1. Upload a new document into the papermerge webapp
  2. Specify a new tag for the document and use "<script>alert(0)</script>" without quotes
  3. Experience a JS popup with 0

grafik

@ciur
Copy link
Owner

ciur commented Nov 29, 2020

fixes for XSS issues so far (work in progress):

  1. for folder titles
  2. for renaming documents/folders
  3. for tags

@ciur ciur removed this from the Version 1.5.1 milestone Nov 29, 2020
@ciur
Copy link
Owner

ciur commented Nov 29, 2020

@l4rm4nd
I released 1.5.1 which contains bug fixes plus partial fixing of this XSS - this is why current issue #228 is referenced in that release.
However fixing all XSS vector attacks requires significantly more changes.
Complete fix for XSS problems will be provided as part of 1.5.2. Release 1.5.2 will be entirely dedicated to XSS problems.
It will be out in a week.

Thank you again for your detailed audit!

@l4rm4nd
Copy link
Author

l4rm4nd commented Nov 29, 2020

Catching and preventing malicious user input such as XSS payloads with a custom regex might work, but is not complete.

I'm not an expert in Django but I remember some built-in escaping functions such as escape() and conditional_escape() which could be used.

from django.utils.html import escape

But I will also test some stuff after your fix release.

@ciur
Copy link
Owner

ciur commented Nov 30, 2020

from django.utils.html import escape

@l4rm4nd, right! I added in several places escape(...) before saving to database.
Release which fixes above mentioned issues is 1.5.2

@l4rm4nd
Copy link
Author

l4rm4nd commented Dec 3, 2020

Identified XSS vectors mitigated by release 1.5.2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working important
Projects
None yet
Development

No branches or pull requests

2 participants