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

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/

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 https://github.com/sunhater/kcfinder/issues/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 :smile:.

🐛 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 :smile:

sunhater / kcfinder