diff --git a/rules/java/lang/path_traversal.yml b/rules/java/lang/path_traversal.yml index 2f2c39fda..07e26fc68 100644 --- a/rules/java/lang/path_traversal.yml +++ b/rules/java/lang/path_traversal.yml @@ -1,74 +1,30 @@ imports: - java_shared_lang_user_input +sanitizer: java_lang_path_traversal_paths_with_user_input_sanitizer patterns: - - pattern: | - new $($<...>$$<...>); + - pattern: $.get($); filters: - - variable: METHOD - regex: \A(java\.io\.)?(File|FileReader|FileWriter|FileInputStream|FileOutputStream)\z - - variable: FILE_USER_INPUT - detection: java_lang_path_traversal_user_input - scope: result - - pattern: | - new $($<...>$$<...>); - filters: - - variable: METHOD - regex: \A(javax\.activation\.)?FileDataSource\z - - variable: FILE_USER_INPUT - detection: java_lang_path_traversal_user_input - scope: result - - pattern: | - $($<...>$$<...>); - filters: - - variable: METHOD - regex: \A(java\.io\.File\.)?(createTempFile|createTempDirectory)\z - - variable: FILE_USER_INPUT - detection: java_lang_path_traversal_user_input - scope: result - - pattern: | - $($<...>$$<...>); - filters: - - variable: METHOD - regex: \A(java\.nio\.file\.Files\.)?(createTempFile|createTempDirectory)\z - - variable: FILE_USER_INPUT - detection: java_lang_path_traversal_user_input - scope: result - - pattern: | - $($<...>$$<...>); - filters: - - variable: METHOD - regex: \A(java\.nio\.file\.Paths\.)?get\z - - variable: FILE_USER_INPUT - detection: java_lang_path_traversal_user_input - scope: result + - variable: PATHS + regex: \A(java\.nio\.file\.)?Paths\z + - variable: USER_INPUT + detection: java_shared_lang_user_input auxiliary: - - id: java_lang_path_traversal_user_input - sanitizer: java_lang_path_traversal_sanitized_input - patterns: - - pattern: $; - filters: - - variable: SHARED_USER_INPUT - detection: java_shared_lang_user_input - scope: cursor - - id: java_lang_path_traversal_sanitized_input + - id: java_lang_path_traversal_paths_with_user_input_sanitizer patterns: - - pattern: FilenameUtils.getName($$<_>) + - $<_>.normalize(); languages: - java -trigger: - match_on: presence metadata: - description: Unsanitized user input in file path + description: Missing protection against path traversal remediation_message: | ## Description - Allowing unsanitized user input in path resolution methods means an attacker could gain access to files and folders outside of the intended scope. + Allowing unsanitized user input in path resolution methods means an attacker could gain access to files and folders outside of the intended scope (path traversal). ## Remediations - ❌ Avoid wherever possible + ❌ Wherever possible, avoid constructing filepaths with user input - ✅ Sanitize user input when resolving paths, for example: - Use `FilenameUtils.getName()` to mitigate against unwanted patterns in the path (such as `\..\..`) + ✅ Use Paths helpers to normalize filepaths. This removes unwanted patterns in the path such as `\..\..` that could lead to path traversal attacks ```java public class Cls extends HttpServlet @@ -77,7 +33,10 @@ metadata: public void handleRequest(HttpServletRequest request, HttpServletResponse response) { String image = request.getParameter("user_profile_picture"); - File file = new File("user/profile/" + FilenameUtils.getName(image)); + + // normalize path + Path imagePath = Paths.get("user/profile/" + FilenameUtils.getName(image)).normalize(); + File file = new File(imagePath.toString()); } } ``` diff --git a/rules/java/lang/path_using_user_input.yml b/rules/java/lang/path_using_user_input.yml new file mode 100644 index 000000000..4150fa40a --- /dev/null +++ b/rules/java/lang/path_using_user_input.yml @@ -0,0 +1,89 @@ +imports: + - java_shared_lang_user_input +patterns: + - pattern: | + new $($<...>$$<...>); + filters: + - variable: METHOD + regex: \A(java\.io\.)?(File|FileReader|FileWriter|FileInputStream|FileOutputStream)\z + - variable: FILE_USER_INPUT + detection: java_lang_path_using_user_input_user_input + scope: result + - pattern: | + new $($<...>$$<...>); + filters: + - variable: METHOD + regex: \A(javax\.activation\.)?FileDataSource\z + - variable: FILE_USER_INPUT + detection: java_lang_path_using_user_input_user_input + scope: result + - pattern: | + $($<...>$$<...>); + filters: + - variable: METHOD + regex: \A(java\.io\.File\.)?(createTempFile|createTempDirectory)\z + - variable: FILE_USER_INPUT + detection: java_lang_path_using_user_input_user_input + scope: result + - pattern: | + $($<...>$$<...>); + filters: + - variable: METHOD + regex: \A(java\.nio\.file\.Files\.)?(createTempFile|createTempDirectory)\z + - variable: FILE_USER_INPUT + detection: java_lang_path_using_user_input_user_input + scope: result + - pattern: | + $($<...>$$<...>); + filters: + - variable: METHOD + regex: \A(java\.nio\.file\.Paths\.)?get\z + - variable: FILE_USER_INPUT + detection: java_lang_path_using_user_input_user_input + scope: result +auxiliary: + - id: java_lang_path_using_user_input_user_input + sanitizer: java_lang_path_using_user_input_sanitized_input + patterns: + - pattern: $; + filters: + - variable: SHARED_USER_INPUT + detection: java_shared_lang_user_input + scope: cursor + - id: java_lang_path_using_user_input_sanitized_input + patterns: + - pattern: FilenameUtils.getName($$<_>) +languages: + - java +trigger: + match_on: presence +metadata: + description: Unsanitized user input in file path + remediation_message: | + ## Description + + Allowing unsanitized user input in path resolution methods means an attacker could influence or control the file name or path used by an application, potentially leading to unauthorized access, data disclosure, or other security issues + + ## Remediations + + ❌ Avoid wherever possible + + ✅ Use a safelist to specify which paths or directories can be accessed, and restrict attempts to access directories that are not whitelisted + + ✅ Sanitize user input when resolving paths. For example, use `FilenameUtils.getName()` to extract just the filename from raw input: + + ```java + public class Cls extends HttpServlet + { + + public void handleRequest(HttpServletRequest request, HttpServletResponse response) + { + String image = request.getParameter("user_profile_picture"); + File file = new File("user/profile/" + FilenameUtils.getName(image)); + } + } + ``` + cwe_id: + - 73 + id: java_lang_path_using_user_input + documentation_url: https://docs.bearer.com/reference/rules/java_lang_path_using_user_input diff --git a/tests/java/lang/path_traversal/__snapshots__/test.js.snap b/tests/java/lang/path_traversal/__snapshots__/test.js.snap deleted file mode 100644 index 9561c8677..000000000 --- a/tests/java/lang/path_traversal/__snapshots__/test.js.snap +++ /dev/null @@ -1,85 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`java_lang_path_traversal cookie_file_traversal 1`] = ` -"{ - "low": [ - { - "cwe_ids": [ - "73" - ], - "id": "java_lang_path_traversal", - "title": "Unsanitized user input in file path", - "description": "## Description\\n\\nAllowing unsanitized user input in path resolution methods means an attacker could gain access to files and folders outside of the intended scope.\\n\\n## Remediations\\n❌ Avoid wherever possible\\n\\n✅ Sanitize user input when resolving paths, for example:\\nUse \`FilenameUtils.getName()\` to mitigate against unwanted patterns in the path (such as \`\\\\..\\\\..\`)\\n\\n\`\`\`java\\n public class Cls extends HttpServlet\\n {\\n\\n public void handleRequest(HttpServletRequest request, HttpServletResponse response)\\n {\\n String image = request.getParameter(\\"user_profile_picture\\");\\n File file = new File(\\"user/profile/\\" + FilenameUtils.getName(image));\\n }\\n }\\n\`\`\`\\n\\n## Resources\\n- [OWASP path traversal](https://owasp.org/www-community/attacks/Path_Traversal)\\n", - "documentation_url": "https://docs.bearer.com/reference/rules/java_lang_path_traversal", - "line_number": 17, - "full_filename": "/tmp/bearer-scan/cookie_file_traversal.java", - "filename": ".", - "source": { - "start": 17, - "end": 17, - "column": { - "start": 21, - "end": 59 - } - }, - "sink": { - "start": 17, - "end": 17, - "column": { - "start": 21, - "end": 59 - }, - "content": "new File(\\"user/profile/\\", userProfile)" - }, - "parent_line_number": 17, - "snippet": "new File(\\"user/profile/\\", userProfile)", - "fingerprint": "0ddbea485a733931cca743266783dadc_0", - "old_fingerprint": "e235348fe3c9ed5201ec4ef44d9dcecd_0", - "code_extract": " File file = new File(\\"user/profile/\\", userProfile);" - } - ] -}" -`; - -exports[`java_lang_path_traversal request_file_traversal 1`] = ` -"{ - "low": [ - { - "cwe_ids": [ - "73" - ], - "id": "java_lang_path_traversal", - "title": "Unsanitized user input in file path", - "description": "## Description\\n\\nAllowing unsanitized user input in path resolution methods means an attacker could gain access to files and folders outside of the intended scope.\\n\\n## Remediations\\n❌ Avoid wherever possible\\n\\n✅ Sanitize user input when resolving paths, for example:\\nUse \`FilenameUtils.getName()\` to mitigate against unwanted patterns in the path (such as \`\\\\..\\\\..\`)\\n\\n\`\`\`java\\n public class Cls extends HttpServlet\\n {\\n\\n public void handleRequest(HttpServletRequest request, HttpServletResponse response)\\n {\\n String image = request.getParameter(\\"user_profile_picture\\");\\n File file = new File(\\"user/profile/\\" + FilenameUtils.getName(image));\\n }\\n }\\n\`\`\`\\n\\n## Resources\\n- [OWASP path traversal](https://owasp.org/www-community/attacks/Path_Traversal)\\n", - "documentation_url": "https://docs.bearer.com/reference/rules/java_lang_path_traversal", - "line_number": 18, - "full_filename": "/tmp/bearer-scan/request_file_traversal.java", - "filename": ".", - "source": { - "start": 18, - "end": 18, - "column": { - "start": 21, - "end": 53 - } - }, - "sink": { - "start": 18, - "end": 18, - "column": { - "start": 21, - "end": 53 - }, - "content": "new File(\\"user/profile/\\", image)" - }, - "parent_line_number": 18, - "snippet": "new File(\\"user/profile/\\", image)", - "fingerprint": "0ddbea485a733931cca743266783dadc_0", - "old_fingerprint": "dacce4d7fc5a813b92dbac426a4a3030_0", - "code_extract": " File file = new File(\\"user/profile/\\", image);" - } - ] -}" -`; - -exports[`java_lang_path_traversal request_file_traversal_sanitized 1`] = `"{}"`; diff --git a/tests/java/lang/path_traversal/test.js b/tests/java/lang/path_traversal/test.js index a1c1ac07c..0abace5c8 100644 --- a/tests/java/lang/path_traversal/test.js +++ b/tests/java/lang/path_traversal/test.js @@ -1,25 +1,18 @@ -const { createInvoker, getEnvironment } = require("../../../helper.js") +const { + createNewInvoker, + getEnvironment, +} = require("../../../helper.js") const { ruleId, ruleFile, testBase } = getEnvironment(__dirname) describe(ruleId, () => { - const invoke = createInvoker(ruleId, ruleFile, testBase) - + const invoke = createNewInvoker(ruleId, ruleFile, testBase) - test("cookie_file_traversal", () => { - const testCase = "cookie_file_traversal.java" - expect(invoke(testCase)).toMatchSnapshot(); - }) - + test("path_traversal", () => { + const testCase = "main.java" - test("request_file_traversal", () => { - const testCase = "request_file_traversal.java" - expect(invoke(testCase)).toMatchSnapshot(); - }) - + const results = invoke(testCase) - test("request_file_traversal_sanitized", () => { - const testCase = "request_file_traversal_sanitized.java" - expect(invoke(testCase)).toMatchSnapshot(); + expect(results.Missing).toEqual([]) + expect(results.Extra).toEqual([]) }) - }) \ No newline at end of file diff --git a/tests/java/lang/path_traversal/testdata/main.java b/tests/java/lang/path_traversal/testdata/main.java new file mode 100644 index 000000000..2c7bffbe8 --- /dev/null +++ b/tests/java/lang/path_traversal/testdata/main.java @@ -0,0 +1,44 @@ +// Use bearer:expected java_lang_path_traversal to flag expected findings + +import javax.servlet.http.HttpServlet; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + +import java.nio.file.Path; +import java.nio.file.Paths; + +public class Foo { + public void bad(HttpServletRequest request, HttpServletResponse response) { + String safeDirectory = "/safe_dir/"; + // bearer:expected java_lang_path_traversal + Path fullPath = Paths.get(request.getParameter("filepath")); + + if (fullPath.startsWith(Paths.get(safeDirectory))) { + // ... + } + } + + public void bad2(HttpServletRequest request, HttpServletResponse response) { + String safeDirectory = "/safe_dir/"; + String filename = request.getParameter("filepath"); + // bearer:expected java_lang_path_traversal + Path fullPath = Paths.get("tmp/user/" + filename); + + if (fullPath.startsWith(Paths.get(safeDirectory))) { + // ... + } + } + + public void ok(HttpServletRequest request, HttpServletResponse response) { + String safeDirectory = "/safe_dir/"; + String filepath = "/tmp/user/bar.txt"; + Path fullPath = Paths.get(request.getParameter("filepath")).normalize(); + Path fullPath = Paths.get(filepath).normalize(); + + if (fullPath.startsWith(Paths.get(safeDirectory))) { + // ... + } + } +} + + diff --git a/tests/java/lang/path_using_user_input/__snapshots__/test.js.snap b/tests/java/lang/path_using_user_input/__snapshots__/test.js.snap new file mode 100644 index 000000000..191862724 --- /dev/null +++ b/tests/java/lang/path_using_user_input/__snapshots__/test.js.snap @@ -0,0 +1,85 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`java_lang_path_using_user_input cookie_file_traversal 1`] = ` +"{ + "low": [ + { + "cwe_ids": [ + "73" + ], + "id": "java_lang_path_using_user_input", + "title": "Unsanitized user input in file path", + "description": "## Description\\n\\nAllowing unsanitized user input in path resolution methods means an attacker could influence or control the file name or path used by an application, potentially leading to unauthorized access, data disclosure, or other security issues\\n\\n## Remediations\\n\\n❌ Avoid wherever possible\\n\\n✅ Use a safelist to specify which paths or directories can be accessed, and restrict attempts to access directories that are not whitelisted\\n\\n✅ Sanitize user input when resolving paths. For example, use \`FilenameUtils.getName()\` to extract just the filename from raw input:\\n\\n\`\`\`java\\n public class Cls extends HttpServlet\\n {\\n\\n public void handleRequest(HttpServletRequest request, HttpServletResponse response)\\n {\\n String image = request.getParameter(\\"user_profile_picture\\");\\n File file = new File(\\"user/profile/\\" + FilenameUtils.getName(image));\\n }\\n }\\n\`\`\`\\n", + "documentation_url": "https://docs.bearer.com/reference/rules/java_lang_path_using_user_input", + "line_number": 17, + "full_filename": "/tmp/bearer-scan/cookie_file_traversal.java", + "filename": ".", + "source": { + "start": 17, + "end": 17, + "column": { + "start": 21, + "end": 59 + } + }, + "sink": { + "start": 17, + "end": 17, + "column": { + "start": 21, + "end": 59 + }, + "content": "new File(\\"user/profile/\\", userProfile)" + }, + "parent_line_number": 17, + "snippet": "new File(\\"user/profile/\\", userProfile)", + "fingerprint": "c64e8061668432989c2de7817c85ecd5_0", + "old_fingerprint": "03f77be4678a0020db2b83ed56723e78_0", + "code_extract": " File file = new File(\\"user/profile/\\", userProfile);" + } + ] +}" +`; + +exports[`java_lang_path_using_user_input request_file_traversal 1`] = ` +"{ + "low": [ + { + "cwe_ids": [ + "73" + ], + "id": "java_lang_path_using_user_input", + "title": "Unsanitized user input in file path", + "description": "## Description\\n\\nAllowing unsanitized user input in path resolution methods means an attacker could influence or control the file name or path used by an application, potentially leading to unauthorized access, data disclosure, or other security issues\\n\\n## Remediations\\n\\n❌ Avoid wherever possible\\n\\n✅ Use a safelist to specify which paths or directories can be accessed, and restrict attempts to access directories that are not whitelisted\\n\\n✅ Sanitize user input when resolving paths. For example, use \`FilenameUtils.getName()\` to extract just the filename from raw input:\\n\\n\`\`\`java\\n public class Cls extends HttpServlet\\n {\\n\\n public void handleRequest(HttpServletRequest request, HttpServletResponse response)\\n {\\n String image = request.getParameter(\\"user_profile_picture\\");\\n File file = new File(\\"user/profile/\\" + FilenameUtils.getName(image));\\n }\\n }\\n\`\`\`\\n", + "documentation_url": "https://docs.bearer.com/reference/rules/java_lang_path_using_user_input", + "line_number": 18, + "full_filename": "/tmp/bearer-scan/request_file_traversal.java", + "filename": ".", + "source": { + "start": 18, + "end": 18, + "column": { + "start": 21, + "end": 53 + } + }, + "sink": { + "start": 18, + "end": 18, + "column": { + "start": 21, + "end": 53 + }, + "content": "new File(\\"user/profile/\\", image)" + }, + "parent_line_number": 18, + "snippet": "new File(\\"user/profile/\\", image)", + "fingerprint": "c64e8061668432989c2de7817c85ecd5_0", + "old_fingerprint": "302071337c4172df531d4fe99a4784ea_0", + "code_extract": " File file = new File(\\"user/profile/\\", image);" + } + ] +}" +`; + +exports[`java_lang_path_using_user_input request_file_traversal_sanitized 1`] = `"{}"`; diff --git a/tests/java/lang/path_using_user_input/test.js b/tests/java/lang/path_using_user_input/test.js new file mode 100644 index 000000000..a1c1ac07c --- /dev/null +++ b/tests/java/lang/path_using_user_input/test.js @@ -0,0 +1,25 @@ +const { createInvoker, getEnvironment } = require("../../../helper.js") +const { ruleId, ruleFile, testBase } = getEnvironment(__dirname) + +describe(ruleId, () => { + const invoke = createInvoker(ruleId, ruleFile, testBase) + + + test("cookie_file_traversal", () => { + const testCase = "cookie_file_traversal.java" + expect(invoke(testCase)).toMatchSnapshot(); + }) + + + test("request_file_traversal", () => { + const testCase = "request_file_traversal.java" + expect(invoke(testCase)).toMatchSnapshot(); + }) + + + test("request_file_traversal_sanitized", () => { + const testCase = "request_file_traversal_sanitized.java" + expect(invoke(testCase)).toMatchSnapshot(); + }) + +}) \ No newline at end of file diff --git a/tests/java/lang/path_traversal/testdata/cookie_file_traversal.java b/tests/java/lang/path_using_user_input/testdata/cookie_file_traversal.java similarity index 100% rename from tests/java/lang/path_traversal/testdata/cookie_file_traversal.java rename to tests/java/lang/path_using_user_input/testdata/cookie_file_traversal.java diff --git a/tests/java/lang/path_traversal/testdata/request_file_traversal.java b/tests/java/lang/path_using_user_input/testdata/request_file_traversal.java similarity index 100% rename from tests/java/lang/path_traversal/testdata/request_file_traversal.java rename to tests/java/lang/path_using_user_input/testdata/request_file_traversal.java diff --git a/tests/java/lang/path_traversal/testdata/request_file_traversal_sanitized.java b/tests/java/lang/path_using_user_input/testdata/request_file_traversal_sanitized.java similarity index 100% rename from tests/java/lang/path_traversal/testdata/request_file_traversal_sanitized.java rename to tests/java/lang/path_using_user_input/testdata/request_file_traversal_sanitized.java