Skip to content
This repository has been archived by the owner on Dec 19, 2023. It is now read-only.

[FIX] XSS through blacklisting and htmlentities() #1

Merged
merged 4 commits into from
Aug 20, 2020

Conversation

Mik317
Copy link

@Mik317 Mik317 commented Aug 14, 2020

Bounty URL: https://www.huntr.dev/bounties/1-packagist-kcfinder

⚙️ Description *

kcfinder was vulnerable against XSS due to a unsafe formatting which occurred in the uploader.php file.
An input provided by the user could be reflected inside the script tag and lead to XSS.

💻 Technical Description *

I used 2 ways to fix the issue:

  • usage of htmlentities() (as suggested on Cross-site Scripting Vulnerability sunhater/kcfinder#180 ) in order to avoid attackers could break the script tag and insert another one/another tag.

  • usage of a blacklist with find and replace approach to avoid the attacker could manipulate the JS syntax and lead to XSS modifying the script purpose

I wanted also to add directly a whitelist to avoid bad crafted function names, but I ended up seeing the regex to validate all the possible function name in JS/ECMA could be really bad and redundant + long (more here: https://stackoverflow.com/questions/2008279/validate-a-javascript-function-name/2008444#2008444). So I opted to replace malicious characters inside the funcname in order to avoid XSS without preventing users to use ascii function names and similar ones 😄.

🐛 Proof of Concept (PoC) *

No POC provided but static analysis is awesome

🔥 Proof of Fix (PoF) *

No steps provided but the fix covers all the cases I could see

👍 User Acceptance Testing (UAT)

Only replaced some malicious characters and called the htmlentities() function on the resulting string 😄

Mik317 added 3 commits August 14, 2020 16:39
Fix JS injection removing invalid `chars` in a `function name` which could result in bypassing `htmlentities()`
Copy link

@mufeedvh mufeedvh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a nice fix! 👏🎉

LGTM

@JamieSlome JamieSlome merged commit 2c73c10 into 418sec:master Aug 20, 2020
@huntr-helper
Copy link

Congratulations Mik317 - your fix has been selected! 🎉

Thanks for being part of the community & helping secure the world's open source code.
If you have any questions, please respond in the comments section. Your bounty is on its way - keep hunting!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants