From 90a9d82b9d9f118c03a3df1a4c683ec1c1ef0946 Mon Sep 17 00:00:00 2001 From: Tony Torralba Date: Thu, 15 Feb 2024 09:54:29 +0100 Subject: [PATCH] Java: Expand ExactPathSanitizer to work on the argument of 'equals' too --- java/ql/lib/semmle/code/java/security/PathSanitizer.qll | 2 +- .../src/change-notes/2024-02-15-path-sanitizer-equals.md | 4 ++++ java/ql/test/library-tests/pathsanitizer/Test.java | 7 +++++++ java/ql/test/library-tests/pathsanitizer/TestKt.kt | 7 +++++++ 4 files changed, 19 insertions(+), 1 deletion(-) create mode 100644 java/ql/src/change-notes/2024-02-15-path-sanitizer-equals.md diff --git a/java/ql/lib/semmle/code/java/security/PathSanitizer.qll b/java/ql/lib/semmle/code/java/security/PathSanitizer.qll index 997a9335168..4ca08f5becc 100644 --- a/java/ql/lib/semmle/code/java/security/PathSanitizer.qll +++ b/java/ql/lib/semmle/code/java/security/PathSanitizer.qll @@ -56,7 +56,7 @@ private predicate exactPathMatchGuard(Guard g, Expr e, boolean branch) { t instanceof StringsKt or t instanceof FilesKt | - e = getVisualQualifier(ma).getUnderlyingExpr() and + e = [getVisualQualifier(ma).getUnderlyingExpr(), getVisualArgument(ma, 0)] and ma.getMethod().getDeclaringType() = t and ma = g and getSourceMethod(ma.getMethod()).hasName(["equals", "equalsIgnoreCase"]) and diff --git a/java/ql/src/change-notes/2024-02-15-path-sanitizer-equals.md b/java/ql/src/change-notes/2024-02-15-path-sanitizer-equals.md new file mode 100644 index 00000000000..3f7fa840fe1 --- /dev/null +++ b/java/ql/src/change-notes/2024-02-15-path-sanitizer-equals.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* The sanitizer for the path injection queries has been improved to handle more cases where `equals` is used to check an exact path match. diff --git a/java/ql/test/library-tests/pathsanitizer/Test.java b/java/ql/test/library-tests/pathsanitizer/Test.java index 61e0f498bd8..718d31dcda6 100644 --- a/java/ql/test/library-tests/pathsanitizer/Test.java +++ b/java/ql/test/library-tests/pathsanitizer/Test.java @@ -25,6 +25,13 @@ public class Test { else sink(source); // $ hasTaintFlow } + { + String source = (String) source(); + if ("/safe/path".equals(source)) + sink(source); // Safe + else + sink(source); // $ hasTaintFlow + } { URI source = (URI) source(); if (source.equals(new URI("http://safe/uri"))) diff --git a/java/ql/test/library-tests/pathsanitizer/TestKt.kt b/java/ql/test/library-tests/pathsanitizer/TestKt.kt index d0a11dac0ee..d26f14ccd60 100644 --- a/java/ql/test/library-tests/pathsanitizer/TestKt.kt +++ b/java/ql/test/library-tests/pathsanitizer/TestKt.kt @@ -25,6 +25,13 @@ class TestKt { else sink(source) // $ hasTaintFlow } + run { + val source = source() as String? + if ("/safe/path".equals(source)) + sink(source) // Safe + else + sink(source) // $ hasTaintFlow + } run { val source = source() as URI? if (source!!.equals(URI("http://safe/uri")))