Security Fix for Cross-site Scripting (XSS) - huntr.dev

[huntr-helper]

https://huntr.dev/users/Mik317 has fixed the Cross-site Scripting (XSS) vulnerability 🔨. Mik317 has been awarded $25 for fixing the vulnerability through the huntr bug bounty program 💵. Think you could fix a vulnerability like this?

Get involved at https://huntr.dev/

Q | A
Version Affected | ALL
Bug Fix | YES
Original Pull Request | 418sec#1
GitHub Issue | #180
Vulnerability README | https://github.com/418sec/huntr/blob/master/bounties/packagist/kcfinder/1/README.md

User Comments:

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 #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 😄