From cdde88d34106a68479f3d22c0af1fc2c43ca4bb5 Mon Sep 17 00:00:00 2001 From: elsapet Date: Wed, 7 Feb 2024 16:19:55 +0200 Subject: [PATCH 1/4] fix: rename existing unsanitized pathname rule --- rules/java/lang/path_using_user_input.yml | 90 +++++++++++++++++++ .../__snapshots__/test.js.snap | 85 ++++++++++++++++++ tests/java/lang/path_using_user_input/test.js | 25 ++++++ .../testdata/cookie_file_traversal.java | 19 ++++ .../testdata/request_file_traversal.java | 21 +++++ .../request_file_traversal_sanitized.java | 20 +++++ 6 files changed, 260 insertions(+) create mode 100644 rules/java/lang/path_using_user_input.yml create mode 100644 tests/java/lang/path_using_user_input/__snapshots__/test.js.snap create mode 100644 tests/java/lang/path_using_user_input/test.js create mode 100644 tests/java/lang/path_using_user_input/testdata/cookie_file_traversal.java create mode 100644 tests/java/lang/path_using_user_input/testdata/request_file_traversal.java create mode 100644 tests/java/lang/path_using_user_input/testdata/request_file_traversal_sanitized.java 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..68a8b8fd9 --- /dev/null +++ b/rules/java/lang/path_using_user_input.yml @@ -0,0 +1,90 @@ +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 gain access to files and folders outside of the intended scope. + + ## Remediations + ❌ Avoid wherever possible + + ✅ Sanitize user input when resolving paths, for example: + Use `FilenameUtils.getName()` to mitigate against unwanted patterns in the path (such as `\..\..`) + + ```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)); + } + } + ``` + + ## Resources + - [OWASP path traversal](https://owasp.org/www-community/attacks/Path_Traversal) + 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_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..174c9f4b8 --- /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 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_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 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_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_using_user_input/testdata/cookie_file_traversal.java b/tests/java/lang/path_using_user_input/testdata/cookie_file_traversal.java new file mode 100644 index 000000000..45fd3a899 --- /dev/null +++ b/tests/java/lang/path_using_user_input/testdata/cookie_file_traversal.java @@ -0,0 +1,19 @@ +package servlets; + +import java.io.File; + +import javax.servlet.http.HttpServlet; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import javax.servlet.http.HttpSession; + +public class Cls extends HttpServlet +{ + + public void handleRequest(HttpServletRequest request, HttpServletResponse response) + { + Cookie[] cookies = request.getCookies(); + String userProfile = cookies[0].getValue() + File file = new File("user/profile/", userProfile); + } +} \ No newline at end of file diff --git a/tests/java/lang/path_using_user_input/testdata/request_file_traversal.java b/tests/java/lang/path_using_user_input/testdata/request_file_traversal.java new file mode 100644 index 000000000..8a2a737c4 --- /dev/null +++ b/tests/java/lang/path_using_user_input/testdata/request_file_traversal.java @@ -0,0 +1,21 @@ +package servlets; + +import java.io.File; + +import javax.servlet.http.HttpServlet; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import javax.servlet.http.HttpSession; + +import org.apache.commons.io.FilenameUtils; + +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/", image); + File okFile = new File("user/profile/", image ? ok : ok); + } +} diff --git a/tests/java/lang/path_using_user_input/testdata/request_file_traversal_sanitized.java b/tests/java/lang/path_using_user_input/testdata/request_file_traversal_sanitized.java new file mode 100644 index 000000000..0e4ac54f5 --- /dev/null +++ b/tests/java/lang/path_using_user_input/testdata/request_file_traversal_sanitized.java @@ -0,0 +1,20 @@ +package servlets; + +import java.io.File; + +import javax.servlet.http.HttpServlet; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import javax.servlet.http.HttpSession; + +import org.apache.commons.io.FilenameUtils; + +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)); + } +} \ No newline at end of file From b5013aab039d59e7286df3123b1ed78a59bfc1ac Mon Sep 17 00:00:00 2001 From: elsapet Date: Wed, 7 Feb 2024 17:08:21 +0200 Subject: [PATCH 2/4] feat: update path traversal rule to be about path traversal --- rules/java/lang/path_traversal.yml | 73 +++++----------- rules/java/lang/path_using_user_input.yml | 11 ++- .../path_traversal/__snapshots__/test.js.snap | 85 ------------------- tests/java/lang/path_traversal/test.js | 27 +++--- .../testdata/cookie_file_traversal.java | 19 ----- .../lang/path_traversal/testdata/main.java | 44 ++++++++++ .../testdata/request_file_traversal.java | 21 ----- .../request_file_traversal_sanitized.java | 20 ----- 8 files changed, 78 insertions(+), 222 deletions(-) delete mode 100644 tests/java/lang/path_traversal/__snapshots__/test.js.snap delete mode 100644 tests/java/lang/path_traversal/testdata/cookie_file_traversal.java create mode 100644 tests/java/lang/path_traversal/testdata/main.java delete mode 100644 tests/java/lang/path_traversal/testdata/request_file_traversal.java delete mode 100644 tests/java/lang/path_traversal/testdata/request_file_traversal_sanitized.java diff --git a/rules/java/lang/path_traversal.yml b/rules/java/lang/path_traversal.yml index 2f2c39fda..d5229e3d8 100644 --- a/rules/java/lang/path_traversal.yml +++ b/rules/java/lang/path_traversal.yml @@ -1,74 +1,36 @@ imports: - java_shared_lang_user_input patterns: - - pattern: | - new $($<...>$$<...>); + - pattern: $.normalize(); 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 + - variable: PATHS_GET + detection: java_lang_path_traversal_paths_with_user_input scope: result +trigger: + match_on: absence + required_detection: java_lang_path_traversal_paths_with_user_input auxiliary: - - id: java_lang_path_traversal_user_input - sanitizer: java_lang_path_traversal_sanitized_input + - id: java_lang_path_traversal_paths_with_user_input patterns: - - pattern: $; + - pattern: $.get($); filters: - - variable: SHARED_USER_INPUT + - variable: PATHS + regex: \A(java\.nio\.file\.)?Paths\z + - variable: USER_INPUT detection: java_shared_lang_user_input - scope: cursor - - id: java_lang_path_traversal_sanitized_input - patterns: - - pattern: FilenameUtils.getName($$<_>) 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 +39,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 index 68a8b8fd9..4150fa40a 100644 --- a/rules/java/lang/path_using_user_input.yml +++ b/rules/java/lang/path_using_user_input.yml @@ -62,13 +62,15 @@ metadata: 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 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 - ✅ Sanitize user input when resolving paths, for example: - Use `FilenameUtils.getName()` to mitigate against unwanted patterns in the path (such as `\..\..`) + ✅ 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 @@ -81,9 +83,6 @@ metadata: } } ``` - - ## Resources - - [OWASP path traversal](https://owasp.org/www-community/attacks/Path_Traversal) cwe_id: - 73 id: 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/cookie_file_traversal.java b/tests/java/lang/path_traversal/testdata/cookie_file_traversal.java deleted file mode 100644 index 45fd3a899..000000000 --- a/tests/java/lang/path_traversal/testdata/cookie_file_traversal.java +++ /dev/null @@ -1,19 +0,0 @@ -package servlets; - -import java.io.File; - -import javax.servlet.http.HttpServlet; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; -import javax.servlet.http.HttpSession; - -public class Cls extends HttpServlet -{ - - public void handleRequest(HttpServletRequest request, HttpServletResponse response) - { - Cookie[] cookies = request.getCookies(); - String userProfile = cookies[0].getValue() - File file = new File("user/profile/", userProfile); - } -} \ 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..0f2f098e6 --- /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_traversal/testdata/request_file_traversal.java b/tests/java/lang/path_traversal/testdata/request_file_traversal.java deleted file mode 100644 index 8a2a737c4..000000000 --- a/tests/java/lang/path_traversal/testdata/request_file_traversal.java +++ /dev/null @@ -1,21 +0,0 @@ -package servlets; - -import java.io.File; - -import javax.servlet.http.HttpServlet; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; -import javax.servlet.http.HttpSession; - -import org.apache.commons.io.FilenameUtils; - -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/", image); - File okFile = new File("user/profile/", image ? ok : ok); - } -} diff --git a/tests/java/lang/path_traversal/testdata/request_file_traversal_sanitized.java b/tests/java/lang/path_traversal/testdata/request_file_traversal_sanitized.java deleted file mode 100644 index 0e4ac54f5..000000000 --- a/tests/java/lang/path_traversal/testdata/request_file_traversal_sanitized.java +++ /dev/null @@ -1,20 +0,0 @@ -package servlets; - -import java.io.File; - -import javax.servlet.http.HttpServlet; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; -import javax.servlet.http.HttpSession; - -import org.apache.commons.io.FilenameUtils; - -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)); - } -} \ No newline at end of file From 6a7fc45335ad060bc69d5fc02ac581a87277ed51 Mon Sep 17 00:00:00 2001 From: elsapet Date: Thu, 8 Feb 2024 10:55:40 +0200 Subject: [PATCH 3/4] fix: use sanitizer over absence trigger --- rules/java/lang/path_traversal.yml | 22 +++++++------------ .../lang/path_traversal/testdata/main.java | 2 +- 2 files changed, 9 insertions(+), 15 deletions(-) diff --git a/rules/java/lang/path_traversal.yml b/rules/java/lang/path_traversal.yml index d5229e3d8..07e26fc68 100644 --- a/rules/java/lang/path_traversal.yml +++ b/rules/java/lang/path_traversal.yml @@ -1,23 +1,17 @@ imports: - java_shared_lang_user_input +sanitizer: java_lang_path_traversal_paths_with_user_input_sanitizer patterns: - - pattern: $.normalize(); + - pattern: $.get($); filters: - - variable: PATHS_GET - detection: java_lang_path_traversal_paths_with_user_input - scope: result -trigger: - match_on: absence - required_detection: java_lang_path_traversal_paths_with_user_input + - variable: PATHS + regex: \A(java\.nio\.file\.)?Paths\z + - variable: USER_INPUT + detection: java_shared_lang_user_input auxiliary: - - id: java_lang_path_traversal_paths_with_user_input + - id: java_lang_path_traversal_paths_with_user_input_sanitizer patterns: - - pattern: $.get($); - filters: - - variable: PATHS - regex: \A(java\.nio\.file\.)?Paths\z - - variable: USER_INPUT - detection: java_shared_lang_user_input + - $<_>.normalize(); languages: - java metadata: diff --git a/tests/java/lang/path_traversal/testdata/main.java b/tests/java/lang/path_traversal/testdata/main.java index 0f2f098e6..2c7bffbe8 100644 --- a/tests/java/lang/path_traversal/testdata/main.java +++ b/tests/java/lang/path_traversal/testdata/main.java @@ -32,7 +32,7 @@ public void bad2(HttpServletRequest request, HttpServletResponse response) { 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(request.getParameter("filepath")).normalize(); Path fullPath = Paths.get(filepath).normalize(); if (fullPath.startsWith(Paths.get(safeDirectory))) { From 135cd86aef62de48df54ab524c0b605092fbbf4a Mon Sep 17 00:00:00 2001 From: elsapet Date: Thu, 8 Feb 2024 13:24:22 +0200 Subject: [PATCH 4/4] fix: update stale snapshot --- .../lang/path_using_user_input/__snapshots__/test.js.snap | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 index 174c9f4b8..191862724 100644 --- a/tests/java/lang/path_using_user_input/__snapshots__/test.js.snap +++ b/tests/java/lang/path_using_user_input/__snapshots__/test.js.snap @@ -9,7 +9,7 @@ exports[`java_lang_path_using_user_input cookie_file_traversal 1`] = ` ], "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 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", + "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", @@ -50,7 +50,7 @@ exports[`java_lang_path_using_user_input request_file_traversal 1`] = ` ], "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 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", + "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",