Finish Partial Path Traversal Query

java/ql/src/Security/CWE/CWE-023/PartialPathTraversal.ql

@@ -10,15 +10,47 @@
  *       external/cwe/cwe-023
  */
 
-import java 
-
+import java
+private import semmle.code.java.dataflow.DataFlow
+private import semmle.code.java.environment.SystemProperty
 
 class MethodStringStartsWith extends Method {
-    MethodStringStartsWith() {
-        this.hasName("startsWith")
-    }
+  MethodStringStartsWith() {
+    this.getDeclaringType() instanceof TypeString and
+    this.hasName("startsWith")
+  }
 }
 
-from MethodAccess ma 
-where ma.getMethod() instanceof MethodStringStartsWith
-select ma, "Partial Path Traversal Vulnerability due to insufficient guard against path traversal"
+class MethodFileGetCanonicalPath extends Method {
+  MethodFileGetCanonicalPath() {
+    this.getDeclaringType() instanceof TypeFile and
+    this.hasName("getCanonicalPath")
+  }
+}
+
+class MethodAccessFileGetCanonicalPath extends MethodAccess {
+  MethodAccessFileGetCanonicalPath() { this.getMethod() instanceof MethodFileGetCanonicalPath }
+}
+
+abstract class FileSeparatorExpr extends Expr { }
+
+class SystemPropFileSeparatorExpr extends FileSeparatorExpr {
+  SystemPropFileSeparatorExpr() { this = getSystemProperty("file.separator") }
+}
+
+class StringLiteralFileSeparatorExpr extends FileSeparatorExpr, StringLiteral {
+  StringLiteralFileSeparatorExpr() { this.getValue() = "/" }
+}
+
+class FileSeparatorAppend extends AddExpr {
+  FileSeparatorAppend() { this.getRightOperand() instanceof FileSeparatorExpr }
+}
+
+predicate isSafe(Expr expr) { DataFlow::localExprFlow(any(FileSeparatorAppend fsa), expr) }
+
+from MethodAccess ma
+where
+  ma.getMethod() instanceof MethodStringStartsWith and
+  DataFlow::localExprFlow(any(MethodAccessFileGetCanonicalPath gcpma), ma.getQualifier()) and
+  not isSafe(ma.getArgument(0))
+select ma, "Partial Path Traversal Vulnerability due to insufficient guard against path traversal"

java/ql/test/query-tests/security/CWE-023/semmle/tests/PartialPathTraversal.expected

@@ -11,7 +11,6 @@
 | PartialPathTraversalTest.java:94:14:94:63 | startsWith(...) | Partial Path Traversal Vulnerability due to insufficient guard against path traversal |
 | PartialPathTraversalTest.java:102:14:102:63 | startsWith(...) | Partial Path Traversal Vulnerability due to insufficient guard against path traversal |
 | PartialPathTraversalTest.java:105:14:105:64 | startsWith(...) | Partial Path Traversal Vulnerability due to insufficient guard against path traversal |
-| PartialPathTraversalTest.java:150:9:150:43 | startsWith(...) | Partial Path Traversal Vulnerability due to insufficient guard against path traversal |
 | PartialPathTraversalTest.java:173:14:173:63 | startsWith(...) | Partial Path Traversal Vulnerability due to insufficient guard against path traversal |
 | PartialPathTraversalTest.java:191:18:191:87 | startsWith(...) | Partial Path Traversal Vulnerability due to insufficient guard against path traversal |
 | PartialPathTraversalTest.java:209:14:209:64 | startsWith(...) | Partial Path Traversal Vulnerability due to insufficient guard against path traversal |

java/ql/test/query-tests/security/CWE-023/semmle/tests/PartialPathTraversalTest.java

@@ -211,6 +211,13 @@ public class PartialPathTraversalTest {
         }
     }
 
+    void foo23(File dir, File parent) throws IOException {
+        String parentCanonical = parent.getCanonicalPath();
+        if (!dir.getCanonicalPath().startsWith(parentCanonical + "/")) {
+            throw new IOException("Invalid directory: " + dir.getCanonicalPath());
+        }
+    }
+
     public void doesNotFlag() {
         "hello".startsWith("goodbye");
     }