From ceae5eef285beb39e79ee187e9e8f9072d0b87a1 Mon Sep 17 00:00:00 2001 From: Tony Torralba Date: Thu, 6 Oct 2022 16:28:31 +0200 Subject: [PATCH] Revert "Decouple from #10177" This reverts commit 7b34b10ceea5aedd0ba9517e80268abb2055dcb6. --- .../java/security/UnsafeContentUriResolution.qll | 4 ++++ java/ql/test/query-tests/security/CWE-441/Test.java | 12 ++++++------ 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/java/ql/lib/semmle/code/java/security/UnsafeContentUriResolution.qll b/java/ql/lib/semmle/code/java/security/UnsafeContentUriResolution.qll index d4bba78ee20..9a12d075e46 100644 --- a/java/ql/lib/semmle/code/java/security/UnsafeContentUriResolution.qll +++ b/java/ql/lib/semmle/code/java/security/UnsafeContentUriResolution.qll @@ -3,6 +3,7 @@ import java private import semmle.code.java.dataflow.TaintTracking private import semmle.code.java.frameworks.android.Android +private import semmle.code.java.security.PathSanitizer /** A URI that gets resolved by a `ContentResolver`. */ abstract class ContentUriResolutionSink extends DataFlow::Node { } @@ -49,6 +50,9 @@ private class UninterestingTypeSanitizer extends ContentUriResolutionSanitizer { } } +private class PathSanitizer extends ContentUriResolutionSanitizer instanceof PathInjectionSanitizer { +} + private class FilenameOnlySanitizer extends ContentUriResolutionSanitizer { FilenameOnlySanitizer() { exists(Method m | this.asExpr().(MethodAccess).getMethod() = m | diff --git a/java/ql/test/query-tests/security/CWE-441/Test.java b/java/ql/test/query-tests/security/CWE-441/Test.java index c88e0d7a65d..0bda0933115 100644 --- a/java/ql/test/query-tests/security/CWE-441/Test.java +++ b/java/ql/test/query-tests/security/CWE-441/Test.java @@ -53,13 +53,13 @@ public class Test extends Activity { Uri uri = (Uri) getIntent().getParcelableExtra("URI_EXTRA"); if (!uri.equals(Uri.parse("content://safe/uri"))) throw new SecurityException(); - contentResolver.openInputStream(uri); // $ SPURIOUS: hasTaintFlow + contentResolver.openInputStream(uri); // Safe } { ContentResolver contentResolver = getContentResolver(); Uri uri = (Uri) getIntent().getParcelableExtra("URI_EXTRA"); validateWithEquals(uri); - contentResolver.openInputStream(uri); // $ SPURIOUS: hasTaintFlow + contentResolver.openInputStream(uri); // Safe } // Allow list checks { @@ -78,13 +78,13 @@ public class Test extends Activity { java.nio.file.FileSystems.getDefault().getPath(path).normalize(); if (!normalized.startsWith("/safe/path")) throw new SecurityException(); - contentResolver.openInputStream(uri); // $ SPURIOUS: hasTaintFlow + contentResolver.openInputStream(uri); // Safe } { ContentResolver contentResolver = getContentResolver(); Uri uri = (Uri) getIntent().getParcelableExtra("URI_EXTRA"); validateWithAllowList(uri); - contentResolver.openInputStream(uri);// $ SPURIOUS: hasTaintFlow + contentResolver.openInputStream(uri); // Safe } // Block list checks { @@ -103,13 +103,13 @@ public class Test extends Activity { java.nio.file.FileSystems.getDefault().getPath(path).normalize(); if (normalized.startsWith("/data")) throw new SecurityException(); - contentResolver.openInputStream(uri); // $ SPURIOUS: hasTaintFlow + contentResolver.openInputStream(uri); // Safe } { ContentResolver contentResolver = getContentResolver(); Uri uri = (Uri) getIntent().getParcelableExtra("URI_EXTRA"); validateWithBlockList(uri); - contentResolver.openInputStream(uri); // $ SPURIOUS: hasTaintFlow + contentResolver.openInputStream(uri); // Safe } } }