Skip to content

Java: Refactor Android Query.qll libraries to new dataflow api #12632

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
category: deprecated
---
* The `WebViewDubuggingQuery` library has been renamed to `WebViewDebuggingQuery` to fix the typo in the file name. `WebViewDubuggingQuery` is now deprecated.
Original file line number Diff line number Diff line change
Expand Up @@ -106,29 +106,29 @@ private class MissingPinningSink extends DataFlow::Node {
}

/** Configuration for finding uses of non trusted URLs. */
private class UntrustedUrlConfig extends TaintTracking::Configuration {
UntrustedUrlConfig() { this = "UntrustedUrlConfig" }

override predicate isSource(DataFlow::Node node) {
private module UntrustedUrlConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node node) {
trustedDomain(_) and
exists(string lit | lit = node.asExpr().(CompileTimeConstantExpr).getStringValue() |
lit.matches("%://%") and // it's a URL
not exists(string dom | trustedDomain(dom) and lit.matches("%" + dom + "%"))
)
}

override predicate isSink(DataFlow::Node node) { node instanceof MissingPinningSink }
predicate isSink(DataFlow::Node node) { node instanceof MissingPinningSink }
}

private module UntrustedUrlFlow = TaintTracking::Global<UntrustedUrlConfig>;

/** Holds if `node` is a network communication call for which certificate pinning is not implemented. */
predicate missingPinning(DataFlow::Node node, string domain) {
isAndroid() and
node instanceof MissingPinningSink and
(
not trustedDomain(_) and domain = ""
or
exists(UntrustedUrlConfig conf, DataFlow::Node src |
conf.hasFlow(src, node) and
exists(DataFlow::Node src |
UntrustedUrlFlow::flow(src, node) and
domain = getDomain(src.asExpr())
)
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,11 @@ import semmle.code.java.dataflow.TaintTracking3
import semmle.code.java.security.AndroidIntentRedirection

/**
* DEPRECATED: Use `IntentRedirectionFlow` instead.
*
* A taint tracking configuration for tainted Intents being used to start Android components.
*/
class IntentRedirectionConfiguration extends TaintTracking::Configuration {
deprecated class IntentRedirectionConfiguration extends TaintTracking::Configuration {
IntentRedirectionConfiguration() { this = "IntentRedirectionConfiguration" }

override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
Expand All @@ -26,31 +28,45 @@ class IntentRedirectionConfiguration extends TaintTracking::Configuration {
}
}

/** A taint tracking configuration for tainted Intents being used to start Android components. */
module IntentRedirectionConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }

predicate isSink(DataFlow::Node sink) { sink instanceof IntentRedirectionSink }

predicate isBarrier(DataFlow::Node sanitizer) { sanitizer instanceof IntentRedirectionSanitizer }

predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
any(IntentRedirectionAdditionalTaintStep c).step(node1, node2)
}
}

/** Tracks the flow of tainted Intents being used to start Android components. */
module IntentRedirectionFlow = TaintTracking::Global<IntentRedirectionConfig>;

/**
* A sanitizer for sinks that receive the original incoming Intent,
* since its component cannot be arbitrarily set.
*/
private class OriginalIntentSanitizer extends IntentRedirectionSanitizer {
OriginalIntentSanitizer() { any(SameIntentBeingRelaunchedConfiguration c).hasFlowTo(this) }
OriginalIntentSanitizer() { SameIntentBeingRelaunchedFlow::flowTo(this) }
}

/**
* Data flow configuration used to discard incoming Intents
* flowing directly to sinks that start Android components.
*/
private class SameIntentBeingRelaunchedConfiguration extends DataFlow2::Configuration {
SameIntentBeingRelaunchedConfiguration() { this = "SameIntentBeingRelaunchedConfiguration" }
private module SameIntentBeingRelaunchedConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }

override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }

override predicate isSink(DataFlow::Node sink) { sink instanceof IntentRedirectionSink }
predicate isSink(DataFlow::Node sink) { sink instanceof IntentRedirectionSink }

override predicate isBarrier(DataFlow::Node barrier) {
predicate isBarrier(DataFlow::Node barrier) {
// Don't discard the Intent if its original component is tainted
barrier instanceof IntentWithTaintedComponent
}

override predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
// Intents being built with the copy constructor from the original Intent are discarded too
exists(ClassInstanceExpr cie |
cie.getConstructedType() instanceof TypeIntent and
Expand All @@ -61,29 +77,31 @@ private class SameIntentBeingRelaunchedConfiguration extends DataFlow2::Configur
}
}

private module SameIntentBeingRelaunchedFlow = DataFlow::Global<SameIntentBeingRelaunchedConfig>;

/** An `Intent` with a tainted component. */
private class IntentWithTaintedComponent extends DataFlow::Node {
IntentWithTaintedComponent() {
exists(IntentSetComponent setExpr, TaintedIntentComponentConf conf |
exists(IntentSetComponent setExpr |
setExpr.getQualifier() = this.asExpr() and
conf.hasFlowTo(DataFlow::exprNode(setExpr.getSink()))
TaintedIntentComponentFlow::flowTo(DataFlow::exprNode(setExpr.getSink()))
)
}
}

/**
* A taint tracking configuration for tainted data flowing to an `Intent`'s component.
*/
private class TaintedIntentComponentConf extends TaintTracking3::Configuration {
TaintedIntentComponentConf() { this = "TaintedIntentComponentConf" }
private module TaintedIntentComponentConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }

override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }

override predicate isSink(DataFlow::Node sink) {
predicate isSink(DataFlow::Node sink) {
any(IntentSetComponent setComponent).getSink() = sink.asExpr()
}
}

private module TaintedIntentComponentFlow = TaintTracking::Global<TaintedIntentComponentConfig>;

/** A call to a method that changes the component of an `Intent`. */
private class IntentSetComponent extends MethodAccess {
int sinkArg;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,25 +14,25 @@ private class OnReceiveMethod extends Method {
}

/** A configuration to detect whether the `action` of an `Intent` is checked. */
private class VerifiedIntentConfig extends DataFlow::Configuration {
VerifiedIntentConfig() { this = "VerifiedIntentConfig" }

override predicate isSource(DataFlow::Node src) {
private module VerifiedIntentConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node src) {
src.asParameter() = any(OnReceiveMethod orm).getIntentParameter()
}

override predicate isSink(DataFlow::Node sink) {
predicate isSink(DataFlow::Node sink) {
exists(MethodAccess ma |
ma.getMethod().hasQualifiedName("android.content", "Intent", "getAction") and
sink.asExpr() = ma.getQualifier()
)
}
}

private module VerifiedIntentFlow = DataFlow::Global<VerifiedIntentConfig>;

/** An `onReceive` method that doesn't verify the action of the intent it receives. */
private class UnverifiedOnReceiveMethod extends OnReceiveMethod {
UnverifiedOnReceiveMethod() {
not any(VerifiedIntentConfig c).hasFlow(DataFlow::parameterNode(this.getIntentParameter()), _)
not VerifiedIntentFlow::flow(DataFlow::parameterNode(this.getIntentParameter()), _)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,23 +91,23 @@ private predicate inputTypeFieldNotCached(Field f) {
}

/** Configuration that finds uses of `setInputType` for non cached fields. */
private class GoodInputTypeConf extends DataFlow::Configuration {
GoodInputTypeConf() { this = "GoodInputTypeConf" }

override predicate isSource(DataFlow::Node node) {
private module GoodInputTypeConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node node) {
inputTypeFieldNotCached(node.asExpr().(FieldAccess).getField())
}

override predicate isSink(DataFlow::Node node) { node.asExpr() = setInputTypeForId(_) }
predicate isSink(DataFlow::Node node) { node.asExpr() = setInputTypeForId(_) }

override predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
exists(OrBitwiseExpr bitOr |
node1.asExpr() = bitOr.getAChildExpr() and
node2.asExpr() = bitOr
)
}
}

private module GoodInputTypeFlow = DataFlow::Global<GoodInputTypeConfig>;

/** Gets a regex indicating that an input field may contain sensitive data. */
private string getInputSensitiveInfoRegex() {
result =
Expand All @@ -130,8 +130,8 @@ AndroidEditableXmlElement getASensitiveCachedInput() {
result.getId().regexpMatch(getInputSensitiveInfoRegex()) and
(
not inputTypeNotCached(result.getInputType()) and
not exists(GoodInputTypeConf conf, DataFlow::Node sink |
conf.hasFlowTo(sink) and
not exists(DataFlow::Node sink |
GoodInputTypeFlow::flowTo(sink) and
sink.asExpr() = setInputTypeForId(result.getId())
)
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,11 @@ import semmle.code.java.security.RequestForgery
import semmle.code.java.security.UnsafeAndroidAccess

/**
* DEPRECATED: Use `FetchUntrustedResourceFlow` instead.
*
* A taint configuration tracking flow from untrusted inputs to a resource fetching call.
*/
class FetchUntrustedResourceConfiguration extends TaintTracking::Configuration {
deprecated class FetchUntrustedResourceConfiguration extends TaintTracking::Configuration {
FetchUntrustedResourceConfiguration() { this = "FetchUntrustedResourceConfiguration" }

override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
Expand All @@ -20,3 +22,19 @@ class FetchUntrustedResourceConfiguration extends TaintTracking::Configuration {
sanitizer instanceof RequestForgerySanitizer
}
}

/**
* A taint configuration tracking flow from untrusted inputs to a resource fetching call.
*/
module FetchUntrustedResourceConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }

predicate isSink(DataFlow::Node sink) { sink instanceof UrlResourceSink }

predicate isBarrier(DataFlow::Node sanitizer) { sanitizer instanceof RequestForgerySanitizer }
}

/**
* Detects taint flow from untrusted inputs to a resource fetching call.
*/
module FetchUntrustedResourceFlow = TaintTracking::Global<FetchUntrustedResourceConfig>;
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
/** Definitions for the Android Webview Debugging Enabled query */

import java
import semmle.code.java.dataflow.DataFlow
import semmle.code.java.controlflow.Guards
import semmle.code.java.security.SecurityTests

/** Holds if `ex` looks like a check that this is a debug build. */
private predicate isDebugCheck(Expr ex) {
exists(Expr subex, string debug |
debug.toLowerCase().matches(["%debug%", "%test%"]) and
subex.getParent*() = ex
|
subex.(VarAccess).getVariable().getName() = debug
or
subex.(MethodAccess).getMethod().hasName("getProperty") and
subex.(MethodAccess).getAnArgument().(CompileTimeConstantExpr).getStringValue() = debug
)
}

/**
* DEPRECATED: Use `WebviewDebugEnabledFlow` instead.
*
* A configuration to find instances of `setWebContentDebuggingEnabled` called with `true` values.
*/
deprecated class WebviewDebugEnabledConfig extends DataFlow::Configuration {
WebviewDebugEnabledConfig() { this = "WebviewDebugEnabledConfig" }

override predicate isSource(DataFlow::Node node) {
node.asExpr().(BooleanLiteral).getBooleanValue() = true
}

override predicate isSink(DataFlow::Node node) {
exists(MethodAccess ma |
ma.getMethod().hasQualifiedName("android.webkit", "WebView", "setWebContentsDebuggingEnabled") and
node.asExpr() = ma.getArgument(0)
)
}

override predicate isBarrier(DataFlow::Node node) {
exists(Guard debug | isDebugCheck(debug) and debug.controls(node.asExpr().getBasicBlock(), _))
or
node.getEnclosingCallable().getDeclaringType() instanceof NonSecurityTestClass
}
}

/** A configuration to find instances of `setWebContentDebuggingEnabled` called with `true` values. */
module WebviewDebugEnabledConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node node) {
node.asExpr().(BooleanLiteral).getBooleanValue() = true
}

predicate isSink(DataFlow::Node node) {
exists(MethodAccess ma |
ma.getMethod().hasQualifiedName("android.webkit", "WebView", "setWebContentsDebuggingEnabled") and
node.asExpr() = ma.getArgument(0)
)
}

predicate isBarrier(DataFlow::Node node) {
exists(Guard debug | isDebugCheck(debug) and debug.controls(node.asExpr().getBasicBlock(), _))
or
node.getEnclosingCallable().getDeclaringType() instanceof NonSecurityTestClass
}
}

/**
* Tracks instances of `setWebContentDebuggingEnabled` with `true` values.
*/
module WebviewDebugEnabledFlow = DataFlow::Global<WebviewDebugEnabledConfig>;
Original file line number Diff line number Diff line change
@@ -1,41 +1,11 @@
/** Definitions for the Android Webview Debugging Enabled query */
/**
* DEPRECATED: Use `semmle.code.java.security.WebviewDebuggingEnabledQuery` instead.
*
* Definitions for the Android Webview Debugging Enabled query
*/

import java
import semmle.code.java.dataflow.DataFlow
import semmle.code.java.controlflow.Guards
import semmle.code.java.security.SecurityTests
private import semmle.code.java.security.WebviewDebuggingEnabledQuery as WebviewDebuggingEnabledQuery

/** Holds if `ex` looks like a check that this is a debug build. */
private predicate isDebugCheck(Expr ex) {
exists(Expr subex, string debug |
debug.toLowerCase().matches(["%debug%", "%test%"]) and
subex.getParent*() = ex
|
subex.(VarAccess).getVariable().getName() = debug
or
subex.(MethodAccess).getMethod().hasName("getProperty") and
subex.(MethodAccess).getAnArgument().(CompileTimeConstantExpr).getStringValue() = debug
)
}

/** A configuration to find instances of `setWebContentDebuggingEnabled` called with `true` values. */
class WebviewDebugEnabledConfig extends DataFlow::Configuration {
WebviewDebugEnabledConfig() { this = "WebviewDebugEnabledConfig" }

override predicate isSource(DataFlow::Node node) {
node.asExpr().(BooleanLiteral).getBooleanValue() = true
}

override predicate isSink(DataFlow::Node node) {
exists(MethodAccess ma |
ma.getMethod().hasQualifiedName("android.webkit", "WebView", "setWebContentsDebuggingEnabled") and
node.asExpr() = ma.getArgument(0)
)
}

override predicate isBarrier(DataFlow::Node node) {
exists(Guard debug | isDebugCheck(debug) and debug.controls(node.asExpr().getBasicBlock(), _))
or
node.getEnclosingCallable().getDeclaringType() instanceof NonSecurityTestClass
}
}
deprecated class WebviewDebugEnabledConfig =
WebviewDebuggingEnabledQuery::WebviewDebugEnabledConfig;
8 changes: 4 additions & 4 deletions java/ql/src/Security/CWE/CWE-489/WebviewDebuggingEnabled.ql
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@
*/

import java
import semmle.code.java.security.WebviewDubuggingEnabledQuery
import DataFlow::PathGraph
import semmle.code.java.security.WebviewDebuggingEnabledQuery
import WebviewDebugEnabledFlow::PathGraph

from WebviewDebugEnabledConfig conf, DataFlow::PathNode source, DataFlow::PathNode sink
where conf.hasFlowPath(source, sink)
from WebviewDebugEnabledFlow::PathNode source, WebviewDebugEnabledFlow::PathNode sink
where WebviewDebugEnabledFlow::flowPath(source, sink)
select sink, source, sink, "Webview debugging is enabled."
Loading