hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-05-01 19:55:15 +02:00
Code Issues Packages Projects Releases Wiki Activity

Update the condition check and use DataFlow in the ql file

Browse Source
This commit is contained in:
luchua-bc
2021-10-04 16:46:44 +00:00
committed by Chris Smowton
parent b0e652a3af
commit 8c2fddb297
4 changed files with 35 additions and 48 deletions
Download Patch File Download Diff File Expand all files Collapse all files

30
java/ql/src/experimental/Security/CWE/CWE-200/AndroidFileIntentSink.qll
View File

@@ -19,30 +19,20 @@ class AsyncTask extends RefType {
AsyncTask() { this.hasQualifiedName("android.os", "AsyncTask") }
}
/** The method that executes `AsyncTask` of Android. */
abstract class ExecuteAsyncTaskMethod extends Method {
/** Returns index of the parameter that is tainted. */
abstract int getParamIndex();
}
/** The `execute` or `executeOnExecutor` method of Android `AsyncTask`. */
class ExecuteAsyncTaskMethod extends Method {
int paramIndex;
/** The `execute` method of Android `AsyncTask`. */
class AsyncTaskExecuteMethod extends ExecuteAsyncTaskMethod {
AsyncTaskExecuteMethod() {
ExecuteAsyncTaskMethod() {
this.getDeclaringType().getSourceDeclaration().getASourceSupertype*() instanceof AsyncTask and
this.getName() = "execute"
(
this.getName() = "execute" and paramIndex = 0
or
this.getName() = "executeOnExecutor" and paramIndex = 1
)
}
override int getParamIndex() { result = 0 }
}
/** The `executeOnExecutor` method of Android `AsyncTask`. */
class AsyncTaskExecuteOnExecutorMethod extends ExecuteAsyncTaskMethod {
AsyncTaskExecuteOnExecutorMethod() {
this.getDeclaringType().getSourceDeclaration().getASourceSupertype*() instanceof AsyncTask and
this.getName() = "executeOnExecutor"
}
override int getParamIndex() { result = 1 }
int getParamIndex() { result = paramIndex }
}
/** The `doInBackground` method of Android `AsyncTask`. */

7
java/ql/src/experimental/Security/CWE/CWE-200/AndroidFileIntentSource.qll
View File

@@ -2,6 +2,7 @@
import java
import semmle.code.java.dataflow.FlowSources
import semmle.code.java.dataflow.TaintTracking2
import semmle.code.java.frameworks.android.Android
/** The `startActivityForResult` method of Android `Activity`. */
@@ -67,14 +68,14 @@ private class AndroidIntentDataModel extends SummaryModelCsv {
}
/** Taint configuration for getting content intent. */
class GetContentIntentConfig extends TaintTracking::Configuration {
class GetContentIntentConfig extends TaintTracking2::Configuration {
GetContentIntentConfig() { this = "GetContentIntentConfig" }
override predicate isSource(DataFlow::Node src) {
override predicate isSource(DataFlow2::Node src) {
exists(GetContentIntent gi | src.asExpr() = gi)
}
override predicate isSink(DataFlow::Node sink) {
override predicate isSink(DataFlow2::Node sink) {
exists(MethodAccess ma |
ma.getMethod() instanceof StartActivityForResultMethod and sink.asExpr() = ma.getArgument(0)
)

36
java/ql/src/experimental/Security/CWE/CWE-200/SensitiveAndroidFileLeak.ql
View File

@@ -4,17 +4,18 @@
* Android configuration file and sensitive user data.
* @kind path-problem
* @id java/sensitive-android-file-leak
* @problem.severity warning
* @tags security
* external/cwe/cwe-200
*/
import java
import semmle.code.java.controlflow.Guards
import AndroidFileIntentSink
import AndroidFileIntentSource
import DataFlow2::PathGraph
import semmle.code.java.dataflow.TaintTracking2
import DataFlow::PathGraph
private class StartsWithSanitizer extends DataFlow2::BarrierGuard {
private class StartsWithSanitizer extends DataFlow::BarrierGuard {
StartsWithSanitizer() { this.(MethodAccess).getMethod().hasName("startsWith") }
override predicate checks(Expr e, boolean branch) {
@@ -27,27 +28,32 @@ private class StartsWithSanitizer extends DataFlow2::BarrierGuard {
}
}
class AndroidFileLeakConfig extends TaintTracking2::Configuration {
class AndroidFileLeakConfig extends TaintTracking::Configuration {
AndroidFileLeakConfig() { this = "AndroidFileLeakConfig" }
/** Holds if it is an access to file intent result. */
override predicate isSource(DataFlow2::Node src) {
/**
* Holds if `src` is a read of some Intent-typed method argument guarded by a check like
* `requestCode == REQUEST_CODE__SELECT_CONTENT_FROM_APPS`, where `requestCode` is the first
* argument to `Activity.onActivityResult`.
*/
override predicate isSource(DataFlow::Node src) {
exists(
AndroidActivityResultInput ai, AndroidFileIntentInput fi, IfStmt ifs, VarAccess intentVar // if (requestCode == REQUEST_CODE__SELECT_CONTENT_FROM_APPS)
AndroidActivityResultInput ai, AndroidFileIntentInput fi, ConditionBlock cb,
VarAccess intentVar
|
ifs.getCondition().getAChildExpr().getAChildExpr().(CompileTimeConstantExpr).getIntValue() =
cb.getCondition().getAChildExpr().(CompileTimeConstantExpr).getIntValue() =
fi.getRequestCode() and
ifs.getCondition().getAChildExpr().getAChildExpr() = ai.getRequestCodeVar() and
cb.getCondition().getAChildExpr() = ai.getRequestCodeVar() and
intentVar.getType() instanceof TypeIntent and
intentVar.(Argument).getAnEnclosingStmt() = ifs.getThen() and
cb.getBasicBlock() = intentVar.(Argument).getAnEnclosingStmt() and
src.asExpr() = intentVar
)
}
/** Holds if it is a sink of file access in Android. */
override predicate isSink(DataFlow2::Node sink) { sink instanceof AndroidFileSink }
override predicate isSink(DataFlow::Node sink) { sink instanceof AndroidFileSink }
override predicate isAdditionalTaintStep(DataFlow2::Node prev, DataFlow2::Node succ) {
override predicate isAdditionalTaintStep(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
@@ -60,18 +66,18 @@ class AndroidFileLeakConfig extends TaintTracking2::Configuration {
csma.getMethod() instanceof ContextStartServiceMethod and
ce.getConstructedType() instanceof TypeIntent and // Intent intent = new Intent(context, FileUploader.class);
ce.getArgument(1).(TypeLiteral).getReferencedType() = ssm.getDeclaringType() and
DataFlow2::localExprFlow(ce, csma.getArgument(0)) and // context.startService(intent);
DataFlow::localExprFlow(ce, csma.getArgument(0)) and // context.startService(intent);
prev.asExpr() = csma.getArgument(0) and
succ.asParameter() = ssm.getParameter(0) // public int onStartCommand(Intent intent, int flags, int startId) {...} in FileUploader
)
}
override predicate isSanitizerGuard(DataFlow2::BarrierGuard guard) {
override predicate isSanitizerGuard(DataFlow::BarrierGuard guard) {
guard instanceof StartsWithSanitizer
}
}
from DataFlow2::PathNode source, DataFlow2::PathNode sink, AndroidFileLeakConfig conf
from DataFlow::PathNode source, DataFlow::PathNode sink, AndroidFileLeakConfig conf
where conf.hasFlowPath(source, sink)
select sink.getNode(), source, sink, "Leaking arbitrary Android file from $@.", source.getNode(),
"this user input"

10
java/ql/test/experimental/query-tests/security/CWE-200/SensitiveAndroidFileLeak.expected
View File

@@ -6,10 +6,6 @@ edges
| FileService.java:25:13:25:51 | makeParamsToExecute(...) : Object[] | FileService.java:40:41:40:55 | params : Object[] |
| FileService.java:25:13:25:51 | makeParamsToExecute(...) [[]] : String | FileService.java:25:13:25:51 | makeParamsToExecute(...) : Object[] |
| FileService.java:25:42:25:50 | localPath : String | FileService.java:25:13:25:51 | makeParamsToExecute(...) [[]] : String |
| FileService.java:25:42:25:50 | localPath : String | FileService.java:32:13:32:28 | sourceUri : String |
| FileService.java:32:13:32:28 | sourceUri : String | FileService.java:35:17:35:25 | sourceUri : String |
| FileService.java:34:20:36:13 | {...} [[]] : String | FileService.java:34:20:36:13 | new Object[] [[]] : String |
| FileService.java:35:17:35:25 | sourceUri : String | FileService.java:34:20:36:13 | {...} [[]] : String |
| FileService.java:40:41:40:55 | params : Object[] | FileService.java:44:33:44:52 | (...)... : Object |
| FileService.java:44:33:44:52 | (...)... : Object | FileService.java:45:53:45:59 | ...[...] |
| LeakFileActivity2.java:16:26:16:31 | intent : Intent | FileService.java:20:31:20:43 | intent : Intent |
@@ -25,10 +21,6 @@ nodes
| FileService.java:25:13:25:51 | makeParamsToExecute(...) : Object[] | semmle.label | makeParamsToExecute(...) : Object[] |
| FileService.java:25:13:25:51 | makeParamsToExecute(...) [[]] : String | semmle.label | makeParamsToExecute(...) [[]] : String |
| FileService.java:25:42:25:50 | localPath : String | semmle.label | localPath : String |
| FileService.java:32:13:32:28 | sourceUri : String | semmle.label | sourceUri : String |
| FileService.java:34:20:36:13 | new Object[] [[]] : String | semmle.label | new Object[] [[]] : String |
| FileService.java:34:20:36:13 | {...} [[]] : String | semmle.label | {...} [[]] : String |
| FileService.java:35:17:35:25 | sourceUri : String | semmle.label | sourceUri : String |
| FileService.java:40:41:40:55 | params : Object[] | semmle.label | params : Object[] |
| FileService.java:44:33:44:52 | (...)... : Object | semmle.label | (...)... : Object |
| FileService.java:45:53:45:59 | ...[...] | semmle.label | ...[...] |
@@ -39,8 +31,6 @@ nodes
| LeakFileActivity.java:19:31:19:53 | getData(...) : Uri | semmle.label | getData(...) : Uri |
| LeakFileActivity.java:21:58:21:72 | streamsToUpload : Uri | semmle.label | streamsToUpload : Uri |
| LeakFileActivity.java:21:58:21:82 | getPath(...) | semmle.label | getPath(...) |
subpaths
| FileService.java:25:42:25:50 | localPath : String | FileService.java:32:13:32:28 | sourceUri : String | FileService.java:34:20:36:13 | new Object[] [[]] : String | FileService.java:25:13:25:51 | makeParamsToExecute(...) [[]] : String |
#select
| FileService.java:45:53:45:59 | ...[...] | LeakFileActivity2.java:16:26:16:31 | intent : Intent | FileService.java:45:53:45:59 | ...[...] | Leaking arbitrary Android file from $@. | LeakFileActivity2.java:16:26:16:31 | intent | this user input |
| LeakFileActivity.java:21:58:21:82 | getPath(...) | LeakFileActivity.java:14:35:14:38 | data : Intent | LeakFileActivity.java:21:58:21:82 | getPath(...) | Leaking arbitrary Android file from $@. | LeakFileActivity.java:14:35:14:38 | data | this user input |