Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(java): add file upload filename rule (CWE-73) #203

Merged
merged 1 commit into from
Jan 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 70 additions & 0 deletions rules/java/lang/file_upload_filename.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
imports:
- java_shared_lang_instance
sanitizer: java_lang_file_upload_filename_sanitized_name
patterns:
- pattern: |
$<UPLOADED_FILE>.getName()
filters:
- variable: UPLOADED_FILE
detection: java_lang_file_upload_filename_uploaded_servlet_file
scope: result
auxiliary:
- id: java_lang_file_upload_filename_sanitized_name
patterns:
- pattern: FilenameUtils.getName($<!>$<_>)
- id: java_lang_file_upload_filename_uploaded_servlet_file
patterns:
- pattern: |
for (FileItem $<FILE_ITEM> : $<UPLOADED_FILES>) {}
focus: FILE_ITEM
filters:
- variable: UPLOADED_FILES
detection: java_lang_file_upload_filename_uploaded_servlet_files
- id: java_lang_file_upload_filename_uploaded_servlet_files
patterns:
- pattern: $<SERVLET>.parseRequest($<SERVLET_REQ>);
filters:
- variable: SERVLET
detection: java_shared_lang_instance
scope: cursor
filters:
- variable: JAVA_SHARED_LANG_INSTANCE_TYPE
regex: \A(org\.apache\.commons\.fileupload\.servlet)?ServletFileUpload\z
- variable: SERVLET_REQ
detection: java_shared_lang_instance
scope: cursor
filters:
- variable: JAVA_SHARED_LANG_INSTANCE_TYPE
regex: \A(java\.servlet\.http\.)?HttpServletRequest\z
languages:
- java
metadata:
description: Unsanitized use of FileUpload filename detected
remediation_message: |
## Description

The unsanitized use of the filename provided by FileUpload could lead to path traversal attacks, since an attacker could manipulate the filename to gain access to unauthorized resources.
Try to avoid referencing filenames that are open to such manipulation, or if it is unavoidable, ensure that the filename is sanitized and that appropriate validation measures are taken.

## Remediations

❌ Avoid wherever possible

✅ Sanitize user input when resolving paths, for example by using `FilenameUtils.getName()` to normalize the path:

```java
ServletFileUpload upload = new ServletFileUpload();
List<FileItem> fileItems = upload.parseRequest(request);

for (FileItem item : fileItems) {
String filename = FilenameUtils.getName(item.getName());
// ...
}
```

## Resources
- [OWASP path traversal](https://owasp.org/www-community/attacks/Path_Traversal)
cwe_id:
- 22
id: java_lang_file_upload_filename
documentation_url: https://docs.bearer.com/reference/rules/java_lang_file_upload_filename
18 changes: 18 additions & 0 deletions tests/java/lang/file_upload_filename/test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
const {
createNewInvoker,
getEnvironment,
} = require("../../../helper.js")
const { ruleId, ruleFile, testBase } = getEnvironment(__dirname)

describe(ruleId, () => {
const invoke = createNewInvoker(ruleId, ruleFile, testBase)

test("file_upload_filename", () => {
const testCase = "main.java"

const results = invoke(testCase)

expect(results.Missing).toEqual([])
expect(results.Extra).toEqual([])
})
})
22 changes: 22 additions & 0 deletions tests/java/lang/file_upload_filename/testdata/main.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
package file;

import org.apache.commons.fileupload.FileItem;
import org.apache.commons.fileupload.FileUploadException;
import org.apache.commons.fileupload.servlet.ServletFileUpload;
import javax.servlet.http.HttpServletRequest;
import java.util.List;

public class foo {
public void bad(HttpServletRequest request) throws FileUploadException {
ServletFileUpload upload = new ServletFileUpload();
List<FileItem> fileItems = upload.parseRequest(request);

for (FileItem item : fileItems) {
// bearer:expected java_lang_file_upload_filename
String badFileName = item.getName();
// ok - sanitized
String okFileName = FilenameUtils.getName(item.getName());
//...
}
}
}
Loading