Refactor SensitiveAndroidFileLeak

This commit is contained in:
Ed Minnix
2023-04-12 12:30:14 -04:00
parent 685a2043a8
commit 074745315c
2 changed files with 20 additions and 22 deletions

View File

@@ -2,7 +2,7 @@
import java
import semmle.code.java.dataflow.FlowSources
import semmle.code.java.dataflow.TaintTracking2
import semmle.code.java.dataflow.TaintTracking
import semmle.code.java.frameworks.android.Android
/** The `startActivityForResult` method of Android's `Activity` class. */
@@ -29,27 +29,25 @@ class GetContentIntent extends ClassInstanceExpr {
}
/** Taint configuration that identifies `GET_CONTENT` `Intent` instances passed to `startActivityForResult`. */
class GetContentIntentConfig extends TaintTracking2::Configuration {
GetContentIntentConfig() { this = "GetContentIntentConfig" }
module GetContentIntentConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node src) { src.asExpr() instanceof GetContentIntent }
override predicate isSource(DataFlow2::Node src) { src.asExpr() instanceof GetContentIntent }
override predicate isSink(DataFlow2::Node sink) {
predicate isSink(DataFlow::Node sink) {
exists(MethodAccess ma |
ma.getMethod() instanceof StartActivityForResultMethod and sink.asExpr() = ma.getArgument(0)
)
}
override predicate allowImplicitRead(DataFlow::Node node, DataFlow::ContentSet content) {
super.allowImplicitRead(node, content)
or
predicate allowImplicitRead(DataFlow::Node node, DataFlow::ContentSet content) {
// Allow the wrapped intent created by Intent.getChooser to be consumed
// by at the sink:
this.isSink(node) and
isSink(node) and
allowIntentExtrasImplicitRead(node, content)
}
}
module GetContentsIntentFlow = TaintTracking::Global<GetContentIntentConfig>;
/** A `GET_CONTENT` `Intent` instances that is passed to `startActivityForResult`. */
class AndroidFileIntentInput extends DataFlow::Node {
MethodAccess ma;
@@ -57,8 +55,8 @@ class AndroidFileIntentInput extends DataFlow::Node {
AndroidFileIntentInput() {
this.asExpr() = ma.getArgument(0) and
ma.getMethod() instanceof StartActivityForResultMethod and
exists(GetContentIntentConfig cc, GetContentIntent gi |
cc.hasFlow(DataFlow::exprNode(gi), DataFlow::exprNode(ma.getArgument(0)))
exists(GetContentIntent gi |
GetContentsIntentFlow::flow(DataFlow::exprNode(gi), DataFlow::exprNode(ma.getArgument(0)))
)
}

View File

@@ -14,7 +14,7 @@ import java
import semmle.code.java.controlflow.Guards
import AndroidFileIntentSink
import AndroidFileIntentSource
import DataFlow::PathGraph
import AndroidFileLeakFlow::PathGraph
private predicate startsWithSanitizer(Guard g, Expr e, boolean branch) {
exists(MethodAccess ma |
@@ -25,16 +25,14 @@ private predicate startsWithSanitizer(Guard g, Expr e, boolean branch) {
)
}
class AndroidFileLeakConfig extends TaintTracking::Configuration {
AndroidFileLeakConfig() { this = "AndroidFileLeakConfig" }
module AndroidFileLeakConfig implements DataFlow::ConfigSig {
/**
* Holds if `src` is a read of some Intent-typed variable guarded by a check like
* `requestCode == someCode`, where `requestCode` is the first
* argument to `Activity.onActivityResult` and `someCode` is
* any request code used in a call to `startActivityForResult(intent, someCode)`.
*/
override predicate isSource(DataFlow::Node src) {
predicate isSource(DataFlow::Node src) {
exists(
OnActivityForResultMethod oafr, ConditionBlock cb, CompileTimeConstantExpr cc,
VarAccess intentVar
@@ -50,9 +48,9 @@ class AndroidFileLeakConfig extends TaintTracking::Configuration {
}
/** Holds if it is a sink of file access in Android. */
override predicate isSink(DataFlow::Node sink) { sink instanceof AndroidFileSink }
predicate isSink(DataFlow::Node sink) { sink instanceof AndroidFileSink }
override predicate isAdditionalTaintStep(DataFlow::Node prev, DataFlow::Node succ) {
predicate isAdditionalFlowStep(DataFlow::Node prev, DataFlow::Node succ) {
exists(MethodAccess aema, AsyncTaskRunInBackgroundMethod arm |
// fileAsyncTask.execute(params) will invoke doInBackground(params) of FileAsyncTask
aema.getQualifier().getType() = arm.getDeclaringType() and
@@ -72,12 +70,14 @@ class AndroidFileLeakConfig extends TaintTracking::Configuration {
)
}
override predicate isSanitizer(DataFlow::Node node) {
predicate isBarrier(DataFlow::Node node) {
node = DataFlow::BarrierGuard<startsWithSanitizer/3>::getABarrierNode()
}
}
from DataFlow::PathNode source, DataFlow::PathNode sink, AndroidFileLeakConfig conf
where conf.hasFlowPath(source, sink)
module AndroidFileLeakFlow = TaintTracking::Global<AndroidFileLeakConfig>;
from AndroidFileLeakFlow::PathNode source, AndroidFileLeakFlow::PathNode sink
where AndroidFileLeakFlow::flowPath(source, sink)
select sink.getNode(), source, sink, "Leaking arbitrary Android file from $@.", source.getNode(),
"this user input"