Skip to content

Commit 02bfa1c

Browse files
luchua-bcsmowton
luchua-bc
authored and
smowton
committed
Optimize the query
1 parent 0621e65 commit 02bfa1c

File tree

4 files changed

+39
-32
lines changed

4 files changed

+39
-32
lines changed

‎java/ql/src/experimental/Security/CWE/CWE-200/AndroidFileIntentSink.qll

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,24 +19,30 @@ class AsyncTask extends RefType {
1919
AsyncTask() { this.hasQualifiedName("android.os", "AsyncTask") }
2020
}
2121

22+
/** The method that executes `AsyncTask` of Android. */
23+
abstract class ExecuteAsyncTaskMethod extends Method {
24+
/** Returns index of the parameter that is tainted. */
25+
abstract int getParamIndex();
26+
}
27+
2228
/** The `execute` method of Android `AsyncTask`. */
23-
class AsyncTaskExecuteMethod extends Method {
29+
class AsyncTaskExecuteMethod extends ExecuteAsyncTaskMethod {
2430
AsyncTaskExecuteMethod() {
2531
this.getDeclaringType().getSourceDeclaration().getASourceSupertype*() instanceof AsyncTask and
2632
this.getName() = "execute"
2733
}
2834

29-
int getParamIndex() { result = 0 }
35+
override int getParamIndex() { result = 0 }
3036
}
3137

3238
/** The `executeOnExecutor` method of Android `AsyncTask`. */
33-
class AsyncTaskExecuteOnExecutorMethod extends Method {
39+
class AsyncTaskExecuteOnExecutorMethod extends ExecuteAsyncTaskMethod {
3440
AsyncTaskExecuteOnExecutorMethod() {
3541
this.getDeclaringType().getSourceDeclaration().getASourceSupertype*() instanceof AsyncTask and
3642
this.getName() = "executeOnExecutor"
3743
}
3844

39-
int getParamIndex() { result = 1 }
45+
override int getParamIndex() { result = 1 }
4046
}
4147

4248
/** The `doInBackground` method of Android `AsyncTask`. */

‎java/ql/src/experimental/Security/CWE/CWE-200/AndroidFileIntentSource.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ class StartActivityForResultMethod extends Method {
1515
/** Android class instance of `GET_CONTENT` intent. */
1616
class GetContentIntent extends ClassInstanceExpr {
1717
GetContentIntent() {
18-
this.getConstructedType().getASupertype*() instanceof TypeIntent and
18+
this.getConstructedType() instanceof TypeIntent and
1919
this.getArgument(0).(CompileTimeConstantExpr).getStringValue() =
2020
"android.intent.action.GET_CONTENT"
2121
or

‎java/ql/src/experimental/Security/CWE/CWE-200/SensitiveAndroidFileLeak.ql

Lines changed: 21 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
* @description Getting file intent from user input without path validation could leak arbitrary
44
* Android configuration file and sensitive user data.
55
* @kind path-problem
6-
* @id java/sensitive_android_file_leak
6+
* @id java/sensitive-android-file-leak
77
* @tags security
88
* external/cwe/cwe-200
99
*/
@@ -14,6 +14,19 @@ import AndroidFileIntentSource
1414
import DataFlow2::PathGraph
1515
import semmle.code.java.dataflow.TaintTracking2
1616

17+
private class StartsWithSanitizer extends DataFlow2::BarrierGuard {
18+
StartsWithSanitizer() { this.(MethodAccess).getMethod().hasName("startsWith") }
19+
20+
override predicate checks(Expr e, boolean branch) {
21+
e =
22+
[
23+
this.(MethodAccess).getQualifier(),
24+
this.(MethodAccess).getQualifier().(MethodAccess).getQualifier()
25+
] and
26+
branch = false
27+
}
28+
}
29+
1730
class AndroidFileLeakConfig extends TaintTracking2::Configuration {
1831
AndroidFileLeakConfig() { this = "AndroidFileLeakConfig" }
1932

@@ -38,37 +51,23 @@ class AndroidFileLeakConfig extends TaintTracking2::Configuration {
3851
exists(MethodAccess aema, AsyncTaskRunInBackgroundMethod arm |
3952
// fileAsyncTask.execute(params) will invoke doInBackground(params) of FileAsyncTask
4053
aema.getQualifier().getType() = arm.getDeclaringType() and
41-
(
42-
aema.getMethod() instanceof AsyncTaskExecuteMethod and
43-
prev.asExpr() = aema.getArgument(0)
44-
or
45-
aema.getMethod() instanceof AsyncTaskExecuteOnExecutorMethod and
46-
prev.asExpr() = aema.getArgument(1)
47-
) and
48-
succ.asExpr() = arm.getParameter(0).getAnAccess()
54+
aema.getMethod() instanceof ExecuteAsyncTaskMethod and
55+
prev.asExpr() = aema.getArgument(aema.getMethod().(ExecuteAsyncTaskMethod).getParamIndex()) and
56+
succ.asParameter() = arm.getParameter(0)
4957
)
5058
or
5159
exists(MethodAccess csma, ServiceOnStartCommandMethod ssm, ClassInstanceExpr ce |
5260
csma.getMethod() instanceof ContextStartServiceMethod and
5361
ce.getConstructedType() instanceof TypeIntent and // Intent intent = new Intent(context, FileUploader.class);
54-
ce.getArgument(1).getType().(ParameterizedType).getTypeArgument(0) = ssm.getDeclaringType() and
62+
ce.getArgument(1).(TypeLiteral).getReferencedType() = ssm.getDeclaringType() and
5563
DataFlow2::localExprFlow(ce, csma.getArgument(0)) and // context.startService(intent);
5664
prev.asExpr() = csma.getArgument(0) and
57-
succ.asExpr() = ssm.getParameter(0).getAnAccess() // public int onStartCommand(Intent intent, int flags, int startId) {...} in FileUploader
65+
succ.asParameter() = ssm.getParameter(0) // public int onStartCommand(Intent intent, int flags, int startId) {...} in FileUploader
5866
)
5967
}
6068

61-
override predicate isSanitizer(DataFlow2::Node node) {
62-
exists(
63-
MethodAccess startsWith // "startsWith" path check
64-
|
65-
startsWith.getMethod().hasName("startsWith") and
66-
(
67-
DataFlow2::localExprFlow(node.asExpr(), startsWith.getQualifier()) or
68-
DataFlow2::localExprFlow(node.asExpr(),
69-
startsWith.getQualifier().(MethodAccess).getQualifier())
70-
)
71-
)
69+
override predicate isSanitizerGuard(DataFlow2::BarrierGuard guard) {
70+
guard instanceof StartsWithSanitizer
7271
}
7372
}
7473

‎java/ql/test/experimental/query-tests/security/CWE-200/SensitiveAndroidFileLeak.expected

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,26 +1,28 @@
11
edges
2+
| FileService.java:20:31:20:43 | intent : Intent | FileService.java:21:28:21:33 | intent : Intent |
3+
| FileService.java:20:31:20:43 | intent : Intent | FileService.java:25:42:25:50 | localPath : String |
24
| FileService.java:21:28:21:33 | intent : Intent | FileService.java:21:28:21:64 | getStringExtra(...) : String |
3-
| FileService.java:21:28:21:33 | intent : Intent | FileService.java:25:42:25:50 | localPath : String |
45
| FileService.java:21:28:21:64 | getStringExtra(...) : String | FileService.java:25:42:25:50 | localPath : String |
5-
| FileService.java:25:13:25:51 | makeParamsToExecute(...) : Object[] | FileService.java:44:44:44:49 | params : Object[] |
6+
| FileService.java:25:13:25:51 | makeParamsToExecute(...) : Object[] | FileService.java:40:41:40:55 | params : Object[] |
67
| FileService.java:25:13:25:51 | makeParamsToExecute(...) [[]] : String | FileService.java:25:13:25:51 | makeParamsToExecute(...) : Object[] |
78
| FileService.java:25:42:25:50 | localPath : String | FileService.java:25:13:25:51 | makeParamsToExecute(...) [[]] : String |
9+
| FileService.java:40:41:40:55 | params : Object[] | FileService.java:44:33:44:52 | (...)... : Object |
810
| FileService.java:44:33:44:52 | (...)... : Object | FileService.java:45:53:45:59 | ...[...] |
9-
| FileService.java:44:44:44:49 | params : Object[] | FileService.java:44:33:44:52 | (...)... : Object |
10-
| LeakFileActivity2.java:16:26:16:31 | intent : Intent | FileService.java:21:28:21:33 | intent : Intent |
11+
| LeakFileActivity2.java:16:26:16:31 | intent : Intent | FileService.java:20:31:20:43 | intent : Intent |
1112
| LeakFileActivity.java:14:35:14:38 | data : Intent | LeakFileActivity.java:18:40:18:59 | contentIntent : Intent |
1213
| LeakFileActivity.java:18:40:18:59 | contentIntent : Intent | LeakFileActivity.java:19:31:19:43 | contentIntent : Intent |
1314
| LeakFileActivity.java:19:31:19:43 | contentIntent : Intent | LeakFileActivity.java:19:31:19:53 | getData(...) : Uri |
1415
| LeakFileActivity.java:19:31:19:53 | getData(...) : Uri | LeakFileActivity.java:21:58:21:72 | streamsToUpload : Uri |
1516
| LeakFileActivity.java:21:58:21:72 | streamsToUpload : Uri | LeakFileActivity.java:21:58:21:82 | getPath(...) |
1617
nodes
18+
| FileService.java:20:31:20:43 | intent : Intent | semmle.label | intent : Intent |
1719
| FileService.java:21:28:21:33 | intent : Intent | semmle.label | intent : Intent |
1820
| FileService.java:21:28:21:64 | getStringExtra(...) : String | semmle.label | getStringExtra(...) : String |
1921
| FileService.java:25:13:25:51 | makeParamsToExecute(...) : Object[] | semmle.label | makeParamsToExecute(...) : Object[] |
2022
| FileService.java:25:13:25:51 | makeParamsToExecute(...) [[]] : String | semmle.label | makeParamsToExecute(...) [[]] : String |
2123
| FileService.java:25:42:25:50 | localPath : String | semmle.label | localPath : String |
24+
| FileService.java:40:41:40:55 | params : Object[] | semmle.label | params : Object[] |
2225
| FileService.java:44:33:44:52 | (...)... : Object | semmle.label | (...)... : Object |
23-
| FileService.java:44:44:44:49 | params : Object[] | semmle.label | params : Object[] |
2426
| FileService.java:45:53:45:59 | ...[...] | semmle.label | ...[...] |
2527
| LeakFileActivity2.java:16:26:16:31 | intent : Intent | semmle.label | intent : Intent |
2628
| LeakFileActivity.java:14:35:14:38 | data : Intent | semmle.label | data : Intent |

0 commit comments

Comments
 (0)