Skip to content

Commit 4f13bf8

Browse files
atorralbaatorralba
authored
Merge pull request #6492 from atorralba/atorralba/android-cleartext-storage-database
Java: Create new query Cleartext storage of sensitive information in Android databases
2 parents 1aa32b0 + 54e8ea5 commit 4f13bf8

25 files changed

+679
-45
lines changed

‎java/ql/lib/semmle/code/java/dataflow/ExternalFlow.qll

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,10 +78,12 @@ private import FlowSummary
7878
private module Frameworks {
7979
private import internal.ContainerFlow
8080
private import semmle.code.java.frameworks.android.Android
81+
private import semmle.code.java.frameworks.android.ContentProviders
8182
private import semmle.code.java.frameworks.android.Intent
8283
private import semmle.code.java.frameworks.android.Notifications
8384
private import semmle.code.java.frameworks.android.Slice
8485
private import semmle.code.java.frameworks.android.SQLite
86+
private import semmle.code.java.frameworks.android.Widget
8587
private import semmle.code.java.frameworks.android.XssSinks
8688
private import semmle.code.java.frameworks.ApacheHttp
8789
private import semmle.code.java.frameworks.apache.Collections

‎java/ql/lib/semmle/code/java/frameworks/Thrift.qll

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ class ThriftIface extends Interface {
2525
this.getFile() instanceof ThriftGeneratedFile
2626
}
2727

28+
/** Gets an implementation of a method of this interface. */
2829
Method getAnImplementingMethod() {
2930
result.getDeclaringType().(Class).getASupertype+() = this and
3031
result.overrides+(this.getAMethod()) and

‎java/ql/lib/semmle/code/java/frameworks/android/Android.qll

Lines changed: 0 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -177,42 +177,6 @@ private class UriModel extends SummaryModelCsv {
177177
}
178178
}
179179

180-
private class ContentProviderSourceModels extends SourceModelCsv {
181-
override predicate row(string row) {
182-
row =
183-
[
184-
// ContentInterface models are here for backwards compatibility (it was removed in API 28)
185-
"android.content;ContentInterface;true;call;(String,String,String,Bundle);;Parameter[0..3];contentprovider",
186-
"android.content;ContentProvider;true;call;(String,String,String,Bundle);;Parameter[0..3];contentprovider",
187-
"android.content;ContentProvider;true;call;(String,String,Bundle);;Parameter[0..2];contentprovider",
188-
"android.content;ContentProvider;true;delete;(Uri,String,String[]);;Parameter[0..2];contentprovider",
189-
"android.content;ContentInterface;true;delete;(Uri,Bundle);;Parameter[0..1];contentprovider",
190-
"android.content;ContentProvider;true;delete;(Uri,Bundle);;Parameter[0..1];contentprovider",
191-
"android.content;ContentInterface;true;getType;(Uri);;Parameter[0];contentprovider",
192-
"android.content;ContentProvider;true;getType;(Uri);;Parameter[0];contentprovider",
193-
"android.content;ContentInterface;true;insert;(Uri,ContentValues,Bundle);;Parameter[0];contentprovider",
194-
"android.content;ContentProvider;true;insert;(Uri,ContentValues,Bundle);;Parameter[0..2];contentprovider",
195-
"android.content;ContentProvider;true;insert;(Uri,ContentValues);;Parameter[0..1];contentprovider",
196-
"android.content;ContentInterface;true;openAssetFile;(Uri,String,CancellationSignal);;Parameter[0];contentprovider",
197-
"android.content;ContentProvider;true;openAssetFile;(Uri,String,CancellationSignal);;Parameter[0];contentprovider",
198-
"android.content;ContentProvider;true;openAssetFile;(Uri,String);;Parameter[0];contentprovider",
199-
"android.content;ContentInterface;true;openTypedAssetFile;(Uri,String,Bundle,CancellationSignal);;Parameter[0..2];contentprovider",
200-
"android.content;ContentProvider;true;openTypedAssetFile;(Uri,String,Bundle,CancellationSignal);;Parameter[0..2];contentprovider",
201-
"android.content;ContentProvider;true;openTypedAssetFile;(Uri,String,Bundle);;Parameter[0..2];contentprovider",
202-
"android.content;ContentInterface;true;openFile;(Uri,String,CancellationSignal);;Parameter[0];contentprovider",
203-
"android.content;ContentProvider;true;openFile;(Uri,String,CancellationSignal);;Parameter[0];contentprovider",
204-
"android.content;ContentProvider;true;openFile;(Uri,String);;Parameter[0];contentprovider",
205-
"android.content;ContentInterface;true;query;(Uri,String[],Bundle,CancellationSignal);;Parameter[0..2];contentprovider",
206-
"android.content;ContentProvider;true;query;(Uri,String[],Bundle,CancellationSignal);;Parameter[0..2];contentprovider",
207-
"android.content;ContentProvider;true;query;(Uri,String[],String,String[],String);;Parameter[0..4];contentprovider",
208-
"android.content;ContentProvider;true;query;(Uri,String[],String,String[],String,CancellationSignal);;Parameter[0..4];contentprovider",
209-
"android.content;ContentInterface;true;update;(Uri,ContentValues,Bundle);;Parameter[0..2];contentprovider",
210-
"android.content;ContentProvider;true;update;(Uri,ContentValues,Bundle);;Parameter[0..2];contentprovider",
211-
"android.content;ContentProvider;true;update;(Uri,ContentValues,String,String[]);;Parameter[0..3];contentprovider"
212-
]
213-
}
214-
}
215-
216180
/** Interface for classes whose instances can be written to and restored from a Parcel. */
217181
class TypeParcelable extends Interface {
218182
TypeParcelable() { this.hasQualifiedName("android.os", "Parcelable") }

‎java/ql/lib/semmle/code/java/frameworks/android/ContentProviders.qll

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
/**
2+
* Provides classes and predicates for working with Content Providers.
3+
*/
4+
5+
import java
6+
import semmle.code.java.dataflow.ExternalFlow
7+
8+
/** The class `android.content.ContentValues`. */
9+
class ContentValues extends Class {
10+
ContentValues() { this.hasQualifiedName("android.content", "ContentValues") }
11+
}
12+
13+
private class ContentProviderSourceModels extends SourceModelCsv {
14+
override predicate row(string row) {
15+
row =
16+
[
17+
// ContentInterface models are here for backwards compatibility (it was removed in API 28)
18+
"android.content;ContentInterface;true;call;(String,String,String,Bundle);;Parameter[0..3];contentprovider",
19+
"android.content;ContentProvider;true;call;(String,String,String,Bundle);;Parameter[0..3];contentprovider",
20+
"android.content;ContentProvider;true;call;(String,String,Bundle);;Parameter[0..2];contentprovider",
21+
"android.content;ContentProvider;true;delete;(Uri,String,String[]);;Parameter[0..2];contentprovider",
22+
"android.content;ContentInterface;true;delete;(Uri,Bundle);;Parameter[0..1];contentprovider",
23+
"android.content;ContentProvider;true;delete;(Uri,Bundle);;Parameter[0..1];contentprovider",
24+
"android.content;ContentInterface;true;getType;(Uri);;Parameter[0];contentprovider",
25+
"android.content;ContentProvider;true;getType;(Uri);;Parameter[0];contentprovider",
26+
"android.content;ContentInterface;true;insert;(Uri,ContentValues,Bundle);;Parameter[0];contentprovider",
27+
"android.content;ContentProvider;true;insert;(Uri,ContentValues,Bundle);;Parameter[0..2];contentprovider",
28+
"android.content;ContentProvider;true;insert;(Uri,ContentValues);;Parameter[0..1];contentprovider",
29+
"android.content;ContentInterface;true;openAssetFile;(Uri,String,CancellationSignal);;Parameter[0];contentprovider",
30+
"android.content;ContentProvider;true;openAssetFile;(Uri,String,CancellationSignal);;Parameter[0];contentprovider",
31+
"android.content;ContentProvider;true;openAssetFile;(Uri,String);;Parameter[0];contentprovider",
32+
"android.content;ContentInterface;true;openTypedAssetFile;(Uri,String,Bundle,CancellationSignal);;Parameter[0..2];contentprovider",
33+
"android.content;ContentProvider;true;openTypedAssetFile;(Uri,String,Bundle,CancellationSignal);;Parameter[0..2];contentprovider",
34+
"android.content;ContentProvider;true;openTypedAssetFile;(Uri,String,Bundle);;Parameter[0..2];contentprovider",
35+
"android.content;ContentInterface;true;openFile;(Uri,String,CancellationSignal);;Parameter[0];contentprovider",
36+
"android.content;ContentProvider;true;openFile;(Uri,String,CancellationSignal);;Parameter[0];contentprovider",
37+
"android.content;ContentProvider;true;openFile;(Uri,String);;Parameter[0];contentprovider",
38+
"android.content;ContentInterface;true;query;(Uri,String[],Bundle,CancellationSignal);;Parameter[0..2];contentprovider",
39+
"android.content;ContentProvider;true;query;(Uri,String[],Bundle,CancellationSignal);;Parameter[0..2];contentprovider",
40+
"android.content;ContentProvider;true;query;(Uri,String[],String,String[],String);;Parameter[0..4];contentprovider",
41+
"android.content;ContentProvider;true;query;(Uri,String[],String,String[],String,CancellationSignal);;Parameter[0..4];contentprovider",
42+
"android.content;ContentInterface;true;update;(Uri,ContentValues,Bundle);;Parameter[0..2];contentprovider",
43+
"android.content;ContentProvider;true;update;(Uri,ContentValues,Bundle);;Parameter[0..2];contentprovider",
44+
"android.content;ContentProvider;true;update;(Uri,ContentValues,String,String[]);;Parameter[0..3];contentprovider"
45+
]
46+
}
47+
}
48+
49+
private class SummaryModels extends SummaryModelCsv {
50+
override predicate row(string row) {
51+
row =
52+
[
53+
"android.content;ContentValues;false;put;;;Argument[0];MapKey of Argument[-1];value",
54+
"android.content;ContentValues;false;put;;;Argument[1];MapValue of Argument[-1];value",
55+
"android.content;ContentValues;false;putAll;;;MapKey of Argument[0];MapKey of Argument[-1];value",
56+
"android.content;ContentValues;false;putAll;;;MapValue of Argument[0];MapValue of Argument[-1];value"
57+
]
58+
}
59+
}

‎java/ql/lib/semmle/code/java/frameworks/android/SQLite.qll

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
/** Provides classes and predicates for working with SQLite databases. */
2+
13
import java
24
import Android
35
import semmle.code.java.dataflow.FlowSteps
@@ -24,6 +26,20 @@ class TypeDatabaseUtils extends Class {
2426
TypeDatabaseUtils() { hasQualifiedName("android.database", "DatabaseUtils") }
2527
}
2628

29+
/**
30+
* The class `android.database.sqlite.SQLiteOpenHelper`.
31+
*/
32+
class TypeSQLiteOpenHelper extends Class {
33+
TypeSQLiteOpenHelper() { this.hasQualifiedName("android.database.sqlite", "SQLiteOpenHelper") }
34+
}
35+
36+
/**
37+
* The class `android.database.sqlite.SQLiteStatement`.
38+
*/
39+
class TypeSQLiteStatement extends Class {
40+
TypeSQLiteStatement() { this.hasQualifiedName("android.database.sqlite", "SQLiteStatement") }
41+
}
42+
2743
private class SQLiteSinkCsv extends SinkModelCsv {
2844
override predicate row(string row) {
2945
row =

‎java/ql/lib/semmle/code/java/frameworks/android/Widget.qll

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
/** Provides classes and predicates for working with Android widgets. */
2+
3+
import java
4+
import semmle.code.java.dataflow.ExternalFlow
5+
import semmle.code.java.dataflow.FlowSources
6+
7+
private class AndroidWidgetSourceModels extends SourceModelCsv {
8+
override predicate row(string row) {
9+
row = "android.widget;EditText;true;getText;;;ReturnValue;android-widget"
10+
}
11+
}
12+
13+
private class DefaultAndroidWidgetSources extends RemoteFlowSource {
14+
DefaultAndroidWidgetSources() { sourceNode(this, "android-widget") }
15+
16+
override string getSourceType() { result = "Android widget source" }
17+
}
18+
19+
private class AndroidWidgetSummaryModels extends SummaryModelCsv {
20+
override predicate row(string row) {
21+
row = "android.widget;EditText;true;getText;;;Argument[-1];ReturnValue;taint"
22+
}
23+
}

‎java/ql/lib/semmle/code/java/frameworks/struts/StrutsAnnotations.qll

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ class StrutsAnnotation extends Annotation {
1515
class StrutsActionAnnotation extends StrutsAnnotation {
1616
StrutsActionAnnotation() { this.getType().hasName("Action") }
1717

18+
/** Gets a callable annotated with this annotation. */
1819
Callable getActionCallable() {
1920
result = this.getAnnotatedElement()
2021
or

‎java/ql/lib/semmle/code/java/security/CleartextStorageAndroidDatabaseQuery.qll

Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
1+
/** Provides classes and predicates to reason about cleartext storage in Android databases. */
2+
3+
import java
4+
import semmle.code.java.dataflow.DataFlow
5+
import semmle.code.java.frameworks.android.ContentProviders
6+
import semmle.code.java.frameworks.android.Intent
7+
import semmle.code.java.frameworks.android.SQLite
8+
import semmle.code.java.security.CleartextStorageQuery
9+
10+
private class LocalDatabaseCleartextStorageSink extends CleartextStorageSink {
11+
LocalDatabaseCleartextStorageSink() { localDatabaseInput(_, this.asExpr()) }
12+
}
13+
14+
private class LocalDatabaseCleartextStorageStep extends CleartextStorageAdditionalTaintStep {
15+
override predicate step(DataFlow::Node n1, DataFlow::Node n2) {
16+
// EditText.getText() return type is parsed as `Object`, so we need to
17+
// add a taint step for `Object.toString` to model `editText.getText().toString()`
18+
exists(MethodAccess ma, Method m |
19+
ma.getMethod() = m and
20+
m.getDeclaringType() instanceof TypeObject and
21+
m.hasName("toString")
22+
|
23+
n1.asExpr() = ma.getQualifier() and n2.asExpr() = ma
24+
)
25+
}
26+
}
27+
28+
/** The creation of an object that can be used to store data in a local database. */
29+
class LocalDatabaseOpenMethodAccess extends Storable, Call {
30+
LocalDatabaseOpenMethodAccess() {
31+
exists(Method m | this.(MethodAccess).getMethod() = m |
32+
m.getDeclaringType().getASupertype*() instanceof TypeSQLiteOpenHelper and
33+
m.hasName("getWritableDatabase")
34+
or
35+
m.getDeclaringType() instanceof TypeSQLiteDatabase and
36+
m.hasName(["create", "openDatabase", "openOrCreateDatabase", "compileStatement"])
37+
or
38+
m.getDeclaringType().getASupertype*() instanceof TypeContext and
39+
m.hasName("openOrCreateDatabase")
40+
)
41+
or
42+
this.(ClassInstanceExpr).getConstructedType() instanceof ContentValues
43+
}
44+
45+
override Expr getAnInput() {
46+
exists(LocalDatabaseFlowConfig config, DataFlow::Node database |
47+
localDatabaseInput(database, result) and
48+
config.hasFlow(DataFlow::exprNode(this), database)
49+
)
50+
}
51+
52+
override Expr getAStore() {
53+
exists(LocalDatabaseFlowConfig config, DataFlow::Node database |
54+
localDatabaseStore(database, result) and
55+
config.hasFlow(DataFlow::exprNode(this), database)
56+
)
57+
}
58+
}
59+
60+
/** A method that is both a database input and a database store. */
61+
private class LocalDatabaseInputStoreMethod extends Method {
62+
LocalDatabaseInputStoreMethod() {
63+
this.getDeclaringType() instanceof TypeSQLiteDatabase and
64+
this.getName().matches("exec%SQL")
65+
}
66+
}
67+
68+
/**
69+
* Holds if `input` is a value being prepared for being stored into the SQLite dataabse `database`.
70+
* This can be done using prepared statements, using the class `ContentValues`, or directly
71+
* appending `input` to a SQL query.
72+
*/
73+
private predicate localDatabaseInput(DataFlow::Node database, Argument input) {
74+
exists(Method m | input.getCall().getCallee() = m |
75+
m instanceof LocalDatabaseInputStoreMethod and
76+
database.asExpr() = input.getCall().getQualifier()
77+
or
78+
m.getDeclaringType() instanceof TypeSQLiteDatabase and
79+
m.hasName("compileStatement") and
80+
database.asExpr() = input.getCall()
81+
or
82+
m.getDeclaringType() instanceof ContentValues and
83+
m.hasName("put") and
84+
input.getPosition() = 1 and
85+
database.asExpr() = input.getCall().getQualifier()
86+
)
87+
}
88+
89+
/**
90+
* Holds if `store` is a method call for storing a previously appended input value,
91+
* either through the use of prepared statements, via the `ContentValues` class, or
92+
* directly executing a raw SQL query.
93+
*/
94+
private predicate localDatabaseStore(DataFlow::Node database, MethodAccess store) {
95+
exists(Method m | store.getMethod() = m |
96+
m instanceof LocalDatabaseInputStoreMethod and
97+
database.asExpr() = store.getQualifier()
98+
or
99+
m.getDeclaringType() instanceof TypeSQLiteDatabase and
100+
m.getName().matches(["insert%", "replace%", "update%"]) and
101+
database.asExpr() = store.getAnArgument() and
102+
database.getType() instanceof ContentValues
103+
or
104+
m.getDeclaringType() instanceof TypeSQLiteStatement and
105+
m.hasName(["executeInsert", "executeUpdateDelete"]) and
106+
database.asExpr() = store.getQualifier()
107+
)
108+
}
109+
110+
private class LocalDatabaseFlowConfig extends DataFlow::Configuration {
111+
LocalDatabaseFlowConfig() { this = "LocalDatabaseFlowConfig" }
112+
113+
override predicate isSource(DataFlow::Node source) {
114+
source.asExpr() instanceof LocalDatabaseOpenMethodAccess
115+
}
116+
117+
override predicate isSink(DataFlow::Node sink) {
118+
localDatabaseInput(sink, _) or
119+
localDatabaseStore(sink, _)
120+
}
121+
122+
override predicate isAdditionalFlowStep(DataFlow::Node n1, DataFlow::Node n2) {
123+
// Adds a step for tracking databases through field flow, that is, a database is opened and
124+
// assigned to a field, and then an input or store method is called on that field elsewhere.
125+
exists(Field f |
126+
f.getType() instanceof TypeSQLiteDatabase and
127+
f.getAnAssignedValue() = n1.asExpr() and
128+
f = n2.asExpr().(FieldRead).getField()
129+
)
130+
}
131+
}

‎java/ql/lib/semmle/code/java/security/SensitiveActions.qll

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,11 @@
1414
import java
1515

1616
private string suspicious() {
17-
result = ["%password%", "%passwd%", "%account%", "%accnt%", "%trusted%"]
17+
result =
18+
[
19+
"%password%", "%passwd%", "pwd", "%account%", "%accnt%", "%trusted%", "%refresh%token%",
20+
"%secret%token"
21+
]
1822
}
1923

2024
private string nonSuspicious() {

‎java/ql/src/Security/CWE/CWE-312/CleartextStorageAndroidDatabase.java

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
public void sqliteStorageUnsafe(Context ctx, String name, String password) {
2+
// BAD - sensitive information saved in cleartext.
3+
SQLiteDatabase db = ctx.openOrCreateDatabase("test", Context.MODE_PRIVATE, null);
4+
db.execSQL("INSERT INTO users VALUES (?, ?)", new String[] {name, password});
5+
}
6+
7+
public void sqliteStorageSafe(Context ctx, String name, String password) {
8+
// GOOD - sensitive information encrypted with a custom method.
9+
SQLiteDatabase db = ctx.openOrCreateDatabase("test", Context.MODE_PRIVATE, null);
10+
db.execSQL("INSERT INTO users VALUES (?, ?)", new String[] {name, encrypt(password)});
11+
}
12+
13+
public void sqlCipherStorageSafe(String name, String password, String databasePassword) {
14+
// GOOD - sensitive information saved using SQLCipher.
15+
net.sqlcipher.database.SQLiteDatabase db =
16+
net.sqlcipher.database.SQLiteDatabase.openOrCreateDatabase("test", databasePassword, null);
17+
db.execSQL("INSERT INTO users VALUES (?, ?)", new String[] {name, password});
18+
}
19+
20+
private static String encrypt(String cleartext) {
21+
// Use an encryption or strong hashing algorithm in the real world.
22+
// The example below just returns a SHA-256 hash.
23+
MessageDigest digest = MessageDigest.getInstance("SHA-256");
24+
byte[] hash = digest.digest(cleartext.getBytes(StandardCharsets.UTF_8));
25+
String encoded = Base64.getEncoder().encodeToString(hash);
26+
return encoded;
27+
}

0 commit comments

Comments
 (0)