First query version requiring sinks to flow to write operations

java/ql/src/Security/CWE/CWE-441/UnsafeContentUriResolution.ql

@@ -0,0 +1,22 @@
+/**
+ * @name Uncontrolled data used in path expression
+ * @description Resolving externally-provided content URIs without validation can allow an attacker
+ *              to access unexpected resources.
+ * @kind path-problem
+ * @problem.severity error
+ * @precision high
+ * @id java/android/unsafe-content-uri-resolution
+ * @tags security
+ *     external/cwe/cwe-441
+ *     external/cwe/cwe-610
+ */
+
+import java
+import UnsafeContentUriResolutionQuery
+import DataFlow::PathGraph
+
+from DataFlow::PathNode src, DataFlow::PathNode sink
+where any(UnsafeContentResolutionConf c).hasFlowPath(src, sink)
+select sink.getNode(), src, sink,
+  "This $@ flows to a ContentResolver method that resolves a URI. The result is then used in a write operation.",
+  src.getNode(), "user input"

java/ql/src/Security/CWE/CWE-441/UnsafeContentUriResolution.qll

@@ -0,0 +1,91 @@
+/** Provides classes to reason about vulnerabilites related to content URIs. */
+
+import java
+import semmle.code.java.dataflow.DataFlow
+import semmle.code.java.frameworks.android.Android
+
+/** A URI that gets resolved by a `ContentResolver`. */
+abstract class ContentUriResolutionSink extends DataFlow::Node {
+  /** Gets the call node that resolves this URI. */
+  abstract DataFlow::Node getCallNode();
+}
+
+/** A sanitizer for content URIs. */
+abstract class ContentUriResolutionSanitizer extends DataFlow::Node { }
+
+/**
+ * A unit class for adding additional taint steps to configurations related to
+ * content URI resolution vulnerabilities.
+ */
+abstract class ContentUriResolutionAdditionalTaintStep extends Unit {
+  /** Holds if the step from `node1` to `node2` should be considered an additional taint step. */
+  abstract predicate step(DataFlow::Node node1, DataFlow::Node node2);
+}
+
+/** The URI argument of a call to a `ContentResolver` URI-opening method. */
+private class DefaultContentUriResolutionSink extends ContentUriResolutionSink {
+  DefaultContentUriResolutionSink() {
+    exists(MethodAccess ma |
+      ma.getMethod() instanceof UriOpeningContentResolverMethod and
+      this.asExpr() = ma.getAnArgument() and
+      this.getType().(RefType).hasQualifiedName("android.net", "Uri")
+    )
+  }
+
+  /** Gets the call node of this argument. */
+  override DataFlow::Node getCallNode() {
+    result = DataFlow::exprNode(this.asExpr().(Argument).getCall())
+  }
+}
+
+private class UninterestingTypeSanitizer extends ContentUriResolutionSanitizer {
+  UninterestingTypeSanitizer() {
+    this.getType() instanceof BoxedType or
+    this.getType() instanceof PrimitiveType or
+    this.getType() instanceof NumberType
+  }
+}
+
+private class FilenameOnlySanitizer extends ContentUriResolutionSanitizer {
+  FilenameOnlySanitizer() {
+    exists(Method m | this.asExpr().(MethodAccess).getMethod() = m |
+      m.hasQualifiedName("java.io", "File", "getName") or
+      m.hasQualifiedName("kotlin.io", "FilesKt", ["getNameWithoutExtension", "getExtension"]) or
+      m.hasQualifiedName("org.apache.commons.io", "FilenameUtils", "getName")
+    )
+  }
+}
+
+/**
+ * A `ContentUriResolutionSink` that flows to an image-decoding function.
+ * Such functions raise exceptions when the input is not a valid image,
+ * which prevents accessing arbitrary non-image files.
+ */
+private class DecodedAsAnImageSanitizer extends ContentUriResolutionSanitizer {
+  DecodedAsAnImageSanitizer() {
+    exists(Argument decodeArg, MethodAccess decode |
+      decode.getArgument(0) = decodeArg and
+      decode
+          .getMethod()
+          .hasQualifiedName("android.graphics", "BitmapFactory",
+            [
+              "decodeByteArray", "decodeFile", "decodeFileDescriptor", "decodeResource",
+              "decodeStream"
+            ])
+    |
+      DataFlow::localFlow(this.(ContentUriResolutionSink).getCallNode(),
+        DataFlow::exprNode(decodeArg))
+    )
+  }
+}
+
+/** A `ContentResolver` method that resolves a URI. */
+private class UriOpeningContentResolverMethod extends Method {
+  UriOpeningContentResolverMethod() {
+    this.hasName([
+        "openInputStream", "openOutputStream", "openAssetFile", "openAssetFileDescriptor",
+        "openFile", "openFileDescriptor", "openTypedAssetFile", "openTypedAssetFileDescriptor",
+      ]) and
+    this.getDeclaringType() instanceof AndroidContentResolver
+  }
+}

java/ql/src/Security/CWE/CWE-441/UnsafeContentUriResolutionQuery.qll

@@ -0,0 +1,61 @@
+/** Provides taint tracking configurations to be used in unsafe content URI resolution queries. */
+
+import java
+import semmle.code.java.dataflow.ExternalFlow
+import semmle.code.java.dataflow.FlowSources
+import semmle.code.java.dataflow.TaintTracking
+import semmle.code.java.dataflow.TaintTracking2
+import UnsafeContentUriResolution
+
+/** A taint-tracking configuration to find paths from remote sources to content URI resolutions. */
+class UnsafeContentResolutionConf extends TaintTracking::Configuration {
+  UnsafeContentResolutionConf() { this = "UnsafeContentResolutionConf" }
+
+  override predicate isSource(DataFlow::Node src) { src instanceof RemoteFlowSource }
+
+  override predicate isSink(DataFlow::Node sink) {
+    flowsToWrite(sink.(ContentUriResolutionSink).getCallNode())
+  }
+
+  override predicate isSanitizer(DataFlow::Node sanitizer) {
+    sanitizer instanceof ContentUriResolutionSanitizer
+  }
+
+  override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
+    any(ContentUriResolutionAdditionalTaintStep s).step(node1, node2)
+  }
+}
+
+/** Holds if `node` flows to a write operation. */
+private predicate flowsToWrite(DataFlow::Node node) { any(FlowsToWriteConfig c).hasFlow(node, _) }
+
+/** A taint-tracking configuration to find paths to write operations. */
+private class FlowsToWriteConfig extends TaintTracking2::Configuration {
+  FlowsToWriteConfig() { this = "FlowsToWriteConfig" }
+
+  override predicate isSource(DataFlow::Node src) {
+    src = any(ContentUriResolutionSink s).getCallNode()
+  }
+
+  override predicate isSink(DataFlow::Node sink) {
+    sinkNode(sink, "create-file")
+    or
+    sinkNode(sink, "write-file")
+    or
+    exists(MethodAccess ma | sink.asExpr() = ma.getArgument(0) |
+      ma.getMethod() instanceof WriteStreamMethod
+    )
+  }
+}
+
+private class WriteStreamMethod extends Method {
+  WriteStreamMethod() {
+    this.getAnOverride*().hasQualifiedName("java.io", "OutputStream", "write")
+    or
+    this.hasQualifiedName("org.apache.commons.io", "IOUtils", "copy")
+    or
+    this.hasQualifiedName("org.springframework.util", ["StreamUtils", "CopyUtils"], "copy")
+    or
+    this.hasQualifiedName("com.google.common.io", ["ByteStreams", "CharStreams"], "copy")
+  }
+}