feat(java): add path traversal rule (CWE-22) (#239)

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 $<METHOD>($<...>$<FILE_USER_INPUT>$<...>);
+  - pattern: $<PATHS>.get($<USER_INPUT>);
     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 $<METHOD>($<...>$<FILE_USER_INPUT>$<...>);
-    filters:
-      - variable: METHOD
-        regex: \A(javax\.activation\.)?FileDataSource\z
-      - variable: FILE_USER_INPUT
-        detection: java_lang_path_traversal_user_input
-        scope: result
-  - pattern: |
-      $<METHOD>($<...>$<FILE_USER_INPUT>$<...>);
-    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: |
-      $<METHOD>($<...>$<FILE_USER_INPUT>$<...>);
-    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: |
-      $<METHOD>($<...>$<FILE_USER_INPUT>$<...>);
-    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: $<SHARED_USER_INPUT>;
-        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());
           }
       }
     ```

rules/java/lang/path_using_user_input.yml

@@ -0,0 +1,89 @@
+imports:
+  - java_shared_lang_user_input
+patterns:
+  - pattern: |
+      new $<METHOD>($<...>$<FILE_USER_INPUT>$<...>);
+    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 $<METHOD>($<...>$<FILE_USER_INPUT>$<...>);
+    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: |
+      $<METHOD>($<...>$<FILE_USER_INPUT>$<...>);
+    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: |
+      $<METHOD>($<...>$<FILE_USER_INPUT>$<...>);
+    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: |
+      $<METHOD>($<...>$<FILE_USER_INPUT>$<...>);
+    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: $<SHARED_USER_INPUT>;
+        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

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([])
   })
-
 })

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))) {
+      // ...
+    }
+  }
+}
+
+