starred/papermerge

Fork 0

mirror of https://github.com/ciur/papermerge.git synced 2026-04-25 12:05:58 +03:00

[GH-ISSUE #314] Cross-Site Scripting (XSS) in Automation Tags #247

New issue

Closed

opened 2026-02-25 21:31:32 +03:00 by kerem · 7 comments

kerem commented

2026-02-25 21:31:32 +03:00

Owner

Originally created by @l4rm4nd on GitHub (Feb 25, 2021).
Original GitHub issue: https://github.com/ciur/papermerge/issues/314

Originally assigned to: @ciur on GitHub.

Version: Papermerge 2.0rc35

Steps to reproduce

Log into Papermerge and visit the "automates" function located at /admin/automate/add/. Insert the following payload into the "Tags" formular and confirm the new tag by inserting a single comma.

<script>alert('XSS');</script>

The JavaScript payload gets instantly executed by the browser, which leads to reflected XSS. If the automation with an XSS payload is saved, the issue will evolve to stored XSS.

If the saved automation is browsed again, e.g. via the following URL path /admin/automate/1, the stored JS payload will be executed again.

Steps to mitigate this issue

As previously, it is recommended to escape all untrusted user input before reflecting or storing the data.

Originally created by @l4rm4nd on GitHub (Feb 25, 2021). Original GitHub issue: https://github.com/ciur/papermerge/issues/314 Originally assigned to: @ciur on GitHub. Version: Papermerge 2.0rc35 ****Steps to reproduce**** Log into Papermerge and visit the "automates" function located at ``/admin/automate/add/``. Insert the following payload into the "Tags" formular and confirm the new tag by inserting a single comma. ```` <script>alert('XSS');</script> ```` The JavaScript payload gets instantly executed by the browser, which leads to reflected XSS. If the automation with an XSS payload is saved, the issue will evolve to stored XSS. If the saved automation is browsed again, e.g. via the following URL path ``/admin/automate/1``, the stored JS payload will be executed again. ![image](https://user-images.githubusercontent.com/21357789/109234761-97475400-77cc-11eb-8a6e-9c62fe1b6fdd.png) ****Steps to mitigate this issue**** As previously, it is recommended to escape all untrusted user input before reflecting or storing the data.

kerem

2026-02-25 21:31:32 +03:00

closed this issue
added the
bug
label

kerem commented

2026-02-25 21:31:32 +03:00

Author

Owner

@ciur commented on GitHub (Feb 26, 2021):

@l4rm4nd, danke! As with other high priority bugs (related specifically to 2.0), I will take care of them over the weekend.

@ciur commented on GitHub (Feb 26, 2021): @l4rm4nd, danke! As with other high priority bugs (related specifically to 2.0), I will take care of them over the weekend.

kerem commented

2026-02-25 21:31:32 +03:00

Author

Owner

@ciur commented on GitHub (Feb 27, 2021):

Here is the fix

just want to confirm:
@l4rm4nd, escaping on client side (i.e. javascript part) is not necessary as long as data is stored (in database) escaped, right?

@ciur commented on GitHub (Feb 27, 2021): [Here is the fix](https://github.com/papermerge/papermerge-core/commit/85cae29845107a6d7e948259dbb3c1bfe0727e07) just want to confirm: @l4rm4nd, escaping on client side (i.e. javascript part) is not necessary as long as data is stored (in database) escaped, right?

kerem commented

2026-02-25 21:31:32 +03:00

Author

Owner

@l4rm4nd commented on GitHub (Feb 27, 2021):

Here is the fix

just want to confirm:
@l4rm4nd, escaping on client side (i.e. javascript part) is not necessary as long as data is stored (in database) escaped, right?

Hey @ciur,

usually it is recommended to do both, input and output sanitization.

Tl;dr: Your fix seems reasonable. As long as all data is properly validated before it's being processed or displayed again, you don't have to escape multiple times. If you cannot ensure that the data in your database is safe (e.g. if it comes from 3rd parties), do an additional output sanitization.

With input validation you ensure that unknown, untrusted user data is always properly validated before it is being stored in your database or directly displayed in the web application. Then, you can basically process all database entries directly without escaping them again, since you've already ensured it is in a valid format. See OWASP.

However, there might be valid points for doing an additional output validation. If an attacker e.g. gains access to the database, he would be able to alter data to contain malicious code. Since you trust all database entries... whoops, there is XSS or other vulnerabilities again. But this is an unlikely scenario, so the risk might be acceptable for not implementing it. However, output validation also makes sense, if you cannot properly validate a database insertion beforehand (e.g. when data comes from third parties that can directly access your database and store data in it). This might not be the case yet, but maybe for future versions.

So best practices are to never trust user input .. and also don't trust your own database entries.

@l4rm4nd commented on GitHub (Feb 27, 2021): > > > [Here is the fix](https://github.com/papermerge/papermerge-core/commit/85cae29845107a6d7e948259dbb3c1bfe0727e07) > > just want to confirm: > @l4rm4nd, escaping on client side (i.e. javascript part) is not necessary as long as data is stored (in database) escaped, right? Hey @ciur, usually it is recommended to do both, input and output sanitization. **Tl;dr:** Your fix seems reasonable. As long as all data is properly validated before it's being processed or displayed again, you don't have to escape multiple times. If you cannot ensure that the data in your database is safe (e.g. if it comes from 3rd parties), do an additional output sanitization. With input validation you ensure that unknown, untrusted user data is always properly validated before it is being stored in your database or directly displayed in the web application. Then, you can basically process all database entries directly without escaping them again, since you've already ensured it is in a valid format. See [OWASP](https://cheatsheetseries.owasp.org/cheatsheets/Input_Validation_Cheat_Sheet.html). However, there might be valid points for doing an additional output validation. If an attacker e.g. gains access to the database, he would be able to alter data to contain malicious code. Since you trust all database entries... whoops, there is XSS or other vulnerabilities again. But this is an unlikely scenario, so the risk might be acceptable for not implementing it. However, output validation also makes sense, if you cannot properly validate a database insertion beforehand (e.g. when data comes from third parties that can directly access your database and store data in it). This might not be the case yet, but maybe for future versions. So best practices are to **never trust user input** .. and also don't trust your own database entries.

kerem commented

2026-02-25 21:31:32 +03:00

Author

Owner

@ciur commented on GitHub (Feb 28, 2021):

@l4rm4nd, thank you for explanation!
Provided fix is now part of 2.0.0rc38 release

@ciur commented on GitHub (Feb 28, 2021): @l4rm4nd, thank you for explanation! Provided fix is now part of [2.0.0rc38 release](https://github.com/ciur/papermerge/releases/tag/v2.0.0rc38)

kerem commented

2026-02-25 21:31:32 +03:00

Author

Owner

@l4rm4nd commented on GitHub (Feb 28, 2021):

Version: Papermerge 2.0rc38

Fix successfully escapes user input after the automation is saved. This prevents stored XSS.

However, inserting a new tag like <script>alert('XSS')</script> still leads to reflected XSS, since the input is not escaped before being displayed. But the likelihood for this exploitation is low.

So you indeed have to escape on the client side too as you mentioned, since the user input is directly printed out without proper filtering. I assumed that Django forms would automatically do some escaping, my bad. Haven't checked your code in detail or tested the fix on my system, sorry!

@l4rm4nd commented on GitHub (Feb 28, 2021): Version: Papermerge 2.0rc38 Fix successfully escapes user input after the automation is saved. This prevents **stored XSS**. ![image](https://user-images.githubusercontent.com/21357789/109437000-e8508580-7a22-11eb-8108-b5c6a67da5d8.png) However, inserting a new tag like `<script>alert('XSS')</script>` still leads to **reflected XSS**, since the input is not escaped before being displayed. But the likelihood for this exploitation is low. ![image](https://user-images.githubusercontent.com/21357789/109437088-644acd80-7a23-11eb-9586-40a0a5058bbb.png) So you indeed have to escape on the client side too as you mentioned, since the user input is directly printed out without proper filtering. I assumed that Django forms would automatically do some escaping, my bad. Haven't checked your code in detail or tested the fix on my system, sorry!

kerem commented

2026-02-25 21:31:32 +03:00

Author

Owner

@ciur commented on GitHub (Mar 8, 2021):

Fixed in latest release, which is 2.0.0rc45

@ciur commented on GitHub (Mar 8, 2021): Fixed in latest release, which is [2.0.0rc45](https://github.com/ciur/papermerge/releases/tag/v2.0.0rc45)

kerem commented

2026-02-25 21:31:32 +03:00

Author

Owner

@l4rm4nd commented on GitHub (Mar 8, 2021):

Seems fixed. Cannot reproduce the XSS vulnerability.

@l4rm4nd commented on GitHub (Mar 8, 2021): Seems fixed. Cannot reproduce the XSS vulnerability. ![image](https://user-images.githubusercontent.com/21357789/110302632-bf6b5a00-7ff9-11eb-8fb6-6de43b183b72.png)