Merge pull request #12735 from JLLeitschuh/doc/JLL/fix-partial-path-documentation

Fix partial path traversal Java example Again

Author: atorralba

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

@@ -1,7 +1,7 @@
 public class PartialPathTraversalBad {
     public void example(File dir, File parent) throws IOException {
         if (!dir.getCanonicalPath().startsWith(parent.getCanonicalPath())) {
-            throw new IOException("Invalid directory: " + dir.getCanonicalPath());
+            throw new IOException("Path traversal attempt: " + dir.getCanonicalPath());
         }
     }
 }

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

@@ -1,7 +1,9 @@
+import java.io.File;
+
 public class PartialPathTraversalGood {
     public void example(File dir, File parent) throws IOException {
-        if (!dir.getCanonicalPath().startsWith(parent.getCanonicalPath() + File.separator)) {
-            throw new IOException("Invalid directory: " + dir.getCanonicalPath());
+        if (!dir.toPath().normalize().startsWith(parent.toPath())) {
+            throw new IOException("Path traversal attempt: " + dir.getCanonicalPath());
         }
     }
 }

java/ql/src/Security/CWE/CWE-023/PartialPathTraversalRemainder.inc.qhelp

@@ -26,10 +26,9 @@ and not just children of <code>parent</code>, which is a security issue.
 
 <p>
 
-In this example, the <code>if</code> statement checks if <code>parent.getCanonicalPath() + File.separator </code> 
-is a prefix of <code>dir.getCanonicalPath()</code>. Because <code>parent.getCanonicalPath() + File.separator</code> is
-indeed slash-terminated, the user supplying <code>dir</code> can only access children of 
-<code>parent</code>, as desired.
+In this example, the <code>if</code> statement checks if <code>parent.toPath()</code> 
+is a prefix of <code>dir.normalize()</code>. Because <code>Path#startsWith</code> does the correct check that 
+<code>dir</code> is a child of <code>parent</code>, users will not be able to access siblings of <code>parent</code>, as desired.
 
 </p>
 

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

@@ -225,6 +225,12 @@ void foo24(File dir, File parent) throws IOException {
         }
     }
 
+    public void doesNotFlagOptimalSafeVersion(File dir, File parent) throws IOException {
+        if (!dir.toPath().normalize().startsWith(parent.toPath())) { // Safe
+            throw new IOException("Path traversal attempt: " + dir.getCanonicalPath());
+        }
+    }
+
     public void doesNotFlag() {
         "hello".startsWith("goodbye");
     }