CWE-434 "Unrestricted Upload of File with Dangerous Type" related rules absence

[NinjaGPT]

Hi CodeQL team, @michaelnebel @egregius313 @felicitymay

Recently my team is working on Java code automated security analysis via CodeQL, However I was aware of that no any "Unrestricted File Upload" related rules in CodeQL. in real world, this is a very common type of vulnerability and can lead to remote code execution. I found a similar opened issue, https://github.com/github/codeql/issues/12473, i wonder that do you have any plan of developing this type of rules?

I also want to clarify, "CWE-434: Unrestricted Upload of File with Dangerous Type", it can lead to "Remote Code Execution", and most of (or fully) risks from java code implement.

The potential risks include:

1. If uploaded filename or target path can be controlled by client side, for example, uploadFilePath = request.getParameter("path"), or fileName = request.getParameter("filename"), then attacker can change the uploadFilePath to any location, or filename = shell.jsp, not only JSP file, also can upload exe, xml, jspx, jspf, php, zip, html, etc, relies on different situations, attacker can launch different types of attacks.

2. In some practices, developers'd like to use blacklist of file extension to validate (sanitizer), however some file extensions often be missed, such as if attacker upload a .html / .htm file which contains malicious javascript code, attacker can launch XSS/phishing attacks via it.

3. For other else cases, in some code implement which will validate the MIME (content-type) or File Signature (MagicByte), unfortunately , all of the sanitizers can be bypassed, because attackers can modify MIME and MagicByte to expected value of application, such as Content-Type: image/jpeg (bypass MIME validation) , or git89a<%JSP Code%> as file's content (gif89a is GIF file's MagicByte).

4. Actually attackers also use other approaches to bypass different types of sanitizers, such as nullbyte injection (shell.jsp%00.jpg), or double extensions ( shell.jjspspto bypass replace extension in blacklist to null string ) , etcetera

References:

CWE-434: Unrestricted Upload of File with Dangerous Type https://cwe.mitre.org/data/definitions/434.html

https://0xn3va.gitbook.io/cheat-sheets/web-application/file-upload-vulnerabilities https://book.hacktricks.xyz/pentesting-web/file-upload https://thehackernews.com/2022/05/critical-rce-bug-reported-in-dotcms.html https://nvd.nist.gov/vuln/detail/CVE-2023-40980

[smowton]

Thanks for the suggestion, we'll add a CWE-434 query for Java to our backlog. I note by the way there is already some coverage in other languages:

https://github.com/github/codeql/blob/934474681df8dda28253cb113cc5622efb936e6a/csharp/ql/src/Input%20Validation/UseOfFileUpload.ql#L12 https://github.com/github/codeql/blob/934474681df8dda28253cb113cc5622efb936e6a/ruby/ql/src/queries/security/cwe-912/HttpToFileAccess.ql#L11 https://github.com/github/codeql/blob/934474681df8dda28253cb113cc5622efb936e6a/javascript/ql/src/Security/CWE-912/HttpToFileAccess.ql#L11

If you're interested in getting into writing CodeQL and created a query for this class of vulnerabilities, perhaps inspired by those existing queries and your observations above, you could submit it for a bounty.

[NinjaGPT]

Thanks for the suggestion, we'll add a CWE-434 query for Java to our backlog. I note by the way there is already some coverage in other languages:

https://github.com/github/codeql/blob/934474681df8dda28253cb113cc5622efb936e6a/csharp/ql/src/Input%20Validation/UseOfFileUpload.ql#L12

https://github.com/github/codeql/blob/934474681df8dda28253cb113cc5622efb936e6a/ruby/ql/src/queries/security/cwe-912/HttpToFileAccess.ql#L11

https://github.com/github/codeql/blob/934474681df8dda28253cb113cc5622efb936e6a/javascript/ql/src/Security/CWE-912/HttpToFileAccess.ql#L11

If you're interested in getting into writing CodeQL and created a query for this class of vulnerabilities, perhaps inspired by those existing queries and your observations above, you could submit it for a bounty.

Hey @smowton , thanks for your response, that is a good news, looking forward new rules, and OK, i will try to become a contributor.