github / codeql

CodeQL: the libraries and queries that power security researchers around the world, as well as code scanning in GitHub Advanced Security
https://codeql.github.com
MIT License
7.49k stars 1.49k forks source link

A security issue that codeql cannot detect #12473

Open jiemodesehn opened 1 year ago

jiemodesehn commented 1 year ago

A security issue that codeql cannot detect

A file upload cannot be detected by codeql using the transferTo function

public static File uploadFile(MultipartFile fileUploader) {
        if (fileUploader != null && !fileUploader.isEmpty()) {
            String fileNameString = fileUploader.getOriginalFilename();
            if (!fileNameString.contains(".")) {
                throw new RuntimeException("文件后缀获取不到");
            } else if (10485760L < fileUploader.getSize()) {
                throw new RuntimeException("文件大小不支持大于10M");
            } else {
                String fileSuffix = fileNameString.substring(fileNameString.lastIndexOf(".") + 1);
                String uuid = UUIDUtils.getUuid();
                String newFileName = uuid + "." + fileSuffix;
                String tmpPath = uploadFilePath + File.separator + newFileName;
                File tmpFile = new File(tmpPath);

                try {
                    fileUploader.transferTo(tmpFile);
                    return tmpFile;
                } catch (Exception var8) {
                    throw new RuntimeException("系统异常:上传失败,请查看日志");
                }
            }
        } else {
            throw new RuntimeException("上传文件不能为空");
        }
}

can you tell me how to solve it

smowton commented 1 year ago

To be clear, is your concern that the filename isn't adequately sanitized and could contain ../../.. and so traverse out of uploadFilePath?

jiemodesehn commented 1 year ago

There is a file upload vulnerability here, you can upload jsp files, does codeql have a corresponding ql script to detect it, I did not detect the vulnerability with the official ql script, thank you

smowton commented 1 year ago

So you're not worried about the filename, but about the contents? If so then that seems to me like a configuration vulnerability (making sure that fileUploadPath is not exposed to users, or that the web server is cautious and does not execute anything in that path).

jiemodesehn commented 1 year ago

I am using codeql to audit a set of cms, its fileUploader is user-controllable, causing a file upload vulnerability, but there is no loophole in codeql detection, but in fact, it is possible to upload a jsp file for getshell, and its source is MultipartFile fileUploader, so is it possible? The script uses codeql to detect the vulnerability, thank you

smowton commented 1 year ago

Isn't this a configuration vulnerability? What would you want the Java code to do to ensure the use case is safe, as opposed to fixing the issue via the web server configuration?

jiemodesehn commented 1 year ago
image

The code on the web end will call Fileutil.uploadFile at the end, and the parameters are user-controllable. Thank you very much for answering this question

max-schaefer commented 1 year ago

你希望得到一个什么样的警报呢?这个问题看起来似乎在于服务器的设置而不是Java程序吗的漏洞。

即使成功得上载了一个.jsp档,它只会存在uploadFilePath目录里边,而不是在一个录攻击者能够自己选的地方。那么,除非服务器注册一个uploadFilePath里边的servlet或uploadFilePath是一个可以执行.jsp档的目录,存在那边的.jsp档没办法执行,所以是无害的。

比如说,假如服务器的设置是这样的:

<servlet>
     <servlet-name>myFoo</servlet-name>
     <jsp-file>uploadFilePath/myJSPfile.jsp</jsp-file>
</servlet>
<servlet-mapping>
     <servlet-name>myFoo</servlet-name>
     <url-pattern>/main</url-pattern>
</servlet-mapping>

这个就危险了,因为攻击者可以存他自己的文件到uploadFilePath/myJSPfile.jsp。但是我们认为这个漏洞在服务器的设置,不在Java的程序里。

NinjaGPT commented 8 months ago

Hi @smowton

I have the same question.

First i want to clarify, this case is belongs to the " 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 includes:

  1. to above case, if uploadFilePath can be controlled by client side, for example, uploadFilePath = request.getParameter("path"), then attacker can change the uploadFilePath to any location, 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. we can see there are no any extension validation for uploaded file, if attacker upload a .html / .html 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).

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