Skip to content

Commit e5b4975

Browse files
aschackmullaschackmull
authored
Merge pull request #4675 from luchua-bc/cleartext-storage-shared-prefs
Java: Query to detect cleartext storage of sensitive information using Android SharedPreferences
2 parents 136e5c9 + 606d094 commit e5b4975

File tree

12 files changed

+1247
-11
lines changed

12 files changed

+1247
-11
lines changed

‎java/ql/src/experimental/Security/CWE/CWE-312/CleartextStorageSharedPrefs.java‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
public void testSetSharedPrefs(Context context, String name, String password)
2+
{
3+
{
4+
// BAD - save sensitive information in cleartext
5+
SharedPreferences sharedPrefs = context.getSharedPreferences("user_prefs", Context.MODE_PRIVATE);
6+
Editor editor = sharedPrefs.edit();
7+
editor.putString("name", name);
8+
editor.putString("password", password);
9+
editor.commit();
10+
}
11+
12+
{
13+
// GOOD - save sensitive information in encrypted format
14+
SharedPreferences sharedPrefs = context.getSharedPreferences("user_prefs", Context.MODE_PRIVATE);
15+
Editor editor = sharedPrefs.edit();
16+
editor.putString("name", encrypt(name));
17+
editor.putString("password", encrypt(password));
18+
editor.commit();
19+
}
20+
21+
{
22+
// GOOD - save sensitive information using the built-in `EncryptedSharedPreferences` class in androidx.
23+
MasterKey masterKey = new MasterKey.Builder(context, MasterKey.DEFAULT_MASTER_KEY_ALIAS)
24+
.setKeyScheme(MasterKey.KeyScheme.AES256_GCM)
25+
.build();
26+
27+
SharedPreferences sharedPreferences = EncryptedSharedPreferences.create(
28+
context,
29+
"secret_shared_prefs",
30+
masterKey,
31+
EncryptedSharedPreferences.PrefKeyEncryptionScheme.AES256_SIV,
32+
EncryptedSharedPreferences.PrefValueEncryptionScheme.AES256_GCM);
33+
34+
SharedPreferences.Editor editor = sharedPreferences.edit();
35+
editor.putString("name", name);
36+
editor.putString("password", password);
37+
editor.commit();
38+
}
39+
}

‎java/ql/src/experimental/Security/CWE/CWE-312/CleartextStorageSharedPrefs.qhelp‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
2+
<qhelp>
3+
<overview>
4+
<p>
5+
<code>SharedPreferences</code> is an Android API that stores application preferences using simple sets of data values. Almost every Android application uses this API. It allows to easily save, alter, and retrieve the values stored in the user's profile. However, sensitive information should not be saved in cleartext. Otherwise it can be accessed by any process or user on rooted devices, or can be disclosed through chained vulnerabilities e.g. unexpected access to its private storage through exposed components.
6+
</p>
7+
</overview>
8+
9+
<recommendation>
10+
<p>
11+
Use the <code>EncryptedSharedPreferences</code> API or other encryption algorithms for storing sensitive information.
12+
</p>
13+
</recommendation>
14+
15+
<example>
16+
<p>
17+
In the first example, sensitive user information is stored in cleartext.
18+
</p>
19+
20+
<p>
21+
In the second and third examples, the code encrypts sensitive information before saving it to the device.
22+
</p>
23+
<sample src="CleartextStorageSharedPrefs.java" />
24+
</example>
25+
26+
<references>
27+
<li>
28+
CWE:
29+
<a href="https://cwe.mitre.org/data/definitions/312.html">CWE-312: Cleartext Storage of Sensitive Information</a>
30+
</li>
31+
<li>
32+
Android Developers:
33+
<a href="https://developer.android.com/topic/security/data">Work with data more securely</a>
34+
</li>
35+
<li>
36+
ProAndroidDev:
37+
<a href="https://proandroiddev.com/encrypted-preferences-in-android-af57a89af7c8">Encrypted Preferences in Android</a>
38+
</li>
39+
</references>
40+
</qhelp>

‎java/ql/src/experimental/Security/CWE/CWE-312/CleartextStorageSharedPrefs.ql‎

Lines changed: 163 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,163 @@
1+
/**
2+
* @name Cleartext storage of sensitive information using `SharedPreferences` on Android
3+
* @description Cleartext Storage of Sensitive Information using SharedPreferences on Android allows access for users with root privileges or unexpected exposure from chained vulnerabilities.
4+
* @kind problem
5+
* @id java/android/cleartext-storage-shared-prefs
6+
* @tags security
7+
* external/cwe/cwe-312
8+
*/
9+
10+
import java
11+
import semmle.code.java.dataflow.DataFlow4
12+
import semmle.code.java.dataflow.DataFlow5
13+
import semmle.code.java.dataflow.TaintTracking
14+
import semmle.code.java.frameworks.android.Intent
15+
import semmle.code.java.frameworks.android.SharedPreferences
16+
import semmle.code.java.security.SensitiveActions
17+
18+
/** Holds if the method call is a setter method of `SharedPreferences`. */
19+
private predicate sharedPreferencesInput(DataFlow::Node sharedPrefs, Expr input) {
20+
exists(MethodAccess m |
21+
m.getMethod() instanceof PutSharedPreferenceMethod and
22+
input = m.getArgument(1) and
23+
not exists(EncryptedValueFlowConfig conf | conf.hasFlow(_, DataFlow::exprNode(input))) and
24+
sharedPrefs.asExpr() = m.getQualifier()
25+
)
26+
}
27+
28+
/** Holds if the method call is the store method of `SharedPreferences`. */
29+
private predicate sharedPreferencesStore(DataFlow::Node sharedPrefs, Expr store) {
30+
exists(MethodAccess m |
31+
m.getMethod() instanceof StoreSharedPreferenceMethod and
32+
store = m and
33+
sharedPrefs.asExpr() = m.getQualifier()
34+
)
35+
}
36+
37+
/** Flow from `SharedPreferences` to either a setter or a store method. */
38+
class SharedPreferencesFlowConfig extends DataFlow::Configuration {
39+
SharedPreferencesFlowConfig() {
40+
this = "CleartextStorageSharedPrefs::SharedPreferencesFlowConfig"
41+
}
42+
43+
override predicate isSource(DataFlow::Node src) {
44+
src.asExpr() instanceof SharedPreferencesEditorMethodAccess
45+
}
46+
47+
override predicate isSink(DataFlow::Node sink) {
48+
sharedPreferencesInput(sink, _) or
49+
sharedPreferencesStore(sink, _)
50+
}
51+
}
52+
53+
/**
54+
* Method call of encrypting sensitive information.
55+
* As there are various implementations of encryption (reversible and non-reversible) from both JDK and third parties, this class simply checks method name to take a best guess to reduce false positives.
56+
*/
57+
class EncryptedSensitiveMethodAccess extends MethodAccess {
58+
EncryptedSensitiveMethodAccess() {
59+
this.getMethod().getName().toLowerCase().matches(["%encrypt%", "%hash%"])
60+
}
61+
}
62+
63+
/** Flow configuration of encrypting sensitive information. */
64+
class EncryptedValueFlowConfig extends DataFlow5::Configuration {
65+
EncryptedValueFlowConfig() { this = "CleartextStorageSharedPrefs::EncryptedValueFlowConfig" }
66+
67+
override predicate isSource(DataFlow5::Node src) {
68+
exists(EncryptedSensitiveMethodAccess ema | src.asExpr() = ema)
69+
}
70+
71+
override predicate isSink(DataFlow5::Node sink) {
72+
exists(MethodAccess ma |
73+
ma.getMethod() instanceof PutSharedPreferenceMethod and
74+
sink.asExpr() = ma.getArgument(1)
75+
)
76+
}
77+
}
78+
79+
/** Flow from the create method of `androidx.security.crypto.EncryptedSharedPreferences` to its instance. */
80+
private class EncryptedSharedPrefFlowConfig extends DataFlow4::Configuration {
81+
EncryptedSharedPrefFlowConfig() {
82+
this = "CleartextStorageSharedPrefs::EncryptedSharedPrefFlowConfig"
83+
}
84+
85+
override predicate isSource(DataFlow4::Node src) {
86+
src.asExpr().(MethodAccess).getMethod() instanceof CreateEncryptedSharedPreferencesMethod
87+
}
88+
89+
override predicate isSink(DataFlow4::Node sink) {
90+
sink.asExpr().getType() instanceof SharedPreferences
91+
}
92+
}
93+
94+
/** The call to get a `SharedPreferences.Editor` object, which can set shared preferences or be stored to device. */
95+
class SharedPreferencesEditorMethodAccess extends MethodAccess {
96+
SharedPreferencesEditorMethodAccess() {
97+
this.getMethod() instanceof GetSharedPreferencesEditorMethod and
98+
not exists(
99+
EncryptedSharedPrefFlowConfig config // not exists `SharedPreferences sharedPreferences = EncryptedSharedPreferences.create(...)`
100+
|
101+
config.hasFlow(_, DataFlow::exprNode(this.getQualifier()))
102+
)
103+
}
104+
105+
/** Gets an input, for example `password` in `editor.putString("password", password);`. */
106+
Expr getAnInput() {
107+
exists(SharedPreferencesFlowConfig conf, DataFlow::Node n |
108+
sharedPreferencesInput(n, result) and
109+
conf.hasFlow(DataFlow::exprNode(this), n)
110+
)
111+
}
112+
113+
/** Gets a store, for example `editor.commit();`. */
114+
Expr getAStore() {
115+
exists(SharedPreferencesFlowConfig conf, DataFlow::Node n |
116+
sharedPreferencesStore(n, result) and
117+
conf.hasFlow(DataFlow::exprNode(this), n)
118+
)
119+
}
120+
}
121+
122+
/**
123+
* Flow from sensitive expressions to shared preferences.
124+
* Note it can be merged into `SensitiveSourceFlowConfig` of `Security/CWE/CWE-312/SensitiveStorage.qll` when this query is promoted from the experimental directory.
125+
*/
126+
private class SensitiveSharedPrefsFlowConfig extends TaintTracking::Configuration {
127+
SensitiveSharedPrefsFlowConfig() {
128+
this = "CleartextStorageSharedPrefs::SensitiveSharedPrefsFlowConfig"
129+
}
130+
131+
override predicate isSource(DataFlow::Node src) { src.asExpr() instanceof SensitiveExpr }
132+
133+
override predicate isSink(DataFlow::Node sink) {
134+
exists(MethodAccess m |
135+
m.getMethod() instanceof PutSharedPreferenceMethod and
136+
sink.asExpr() = m.getArgument(1)
137+
)
138+
}
139+
}
140+
141+
/** Class for shared preferences that may contain 'sensitive' information */
142+
class SensitiveSharedPrefsSource extends Expr {
143+
SensitiveSharedPrefsSource() {
144+
// SensitiveExpr is abstract, this lets us inherit from it without
145+
// being a technical subclass
146+
this instanceof SensitiveExpr
147+
}
148+
149+
/** Holds if this source flows to the `sink`. */
150+
predicate flowsTo(Expr sink) {
151+
exists(SensitiveSharedPrefsFlowConfig conf |
152+
conf.hasFlow(DataFlow::exprNode(this), DataFlow::exprNode(sink))
153+
)
154+
}
155+
}
156+
157+
from SensitiveSharedPrefsSource data, SharedPreferencesEditorMethodAccess s, Expr input, Expr store
158+
where
159+
input = s.getAnInput() and
160+
store = s.getAStore() and
161+
data.flowsTo(input)
162+
select store, "'SharedPreferences' class $@ containing $@ is stored here. Data was added $@.", s,
163+
s.toString(), data, "sensitive data", input, "here"

‎java/ql/src/semmle/code/java/frameworks/android/SharedPreferences.qll‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
/** Provides classes related to `android.content.SharedPreferences`. */
2+
3+
import java
4+
5+
/** The interface `android.content.SharedPreferences`. */
6+
class SharedPreferences extends Interface {
7+
SharedPreferences() { this.hasQualifiedName("android.content", "SharedPreferences") }
8+
}
9+
10+
/** The class `androidx.security.crypto.EncryptedSharedPreferences`, which implements `SharedPreferences` with encryption support. */
11+
class EncryptedSharedPreferences extends Class {
12+
EncryptedSharedPreferences() {
13+
this.hasQualifiedName("androidx.security.crypto", "EncryptedSharedPreferences")
14+
}
15+
}
16+
17+
/** The `create` method of `androidx.security.crypto.EncryptedSharedPreferences`. */
18+
class CreateEncryptedSharedPreferencesMethod extends Method {
19+
CreateEncryptedSharedPreferencesMethod() {
20+
this.getDeclaringType() instanceof EncryptedSharedPreferences and
21+
this.hasName("create")
22+
}
23+
}
24+
25+
/** The method `android.content.SharedPreferences::edit`, which returns an `android.content.SharedPreferences.Editor`. */
26+
class GetSharedPreferencesEditorMethod extends Method {
27+
GetSharedPreferencesEditorMethod() {
28+
this.getDeclaringType() instanceof SharedPreferences and
29+
this.hasName("edit") and
30+
this.getReturnType() instanceof SharedPreferencesEditor
31+
}
32+
}
33+
34+
/** The interface `android.content.SharedPreferences.Editor`. */
35+
class SharedPreferencesEditor extends Interface {
36+
SharedPreferencesEditor() { this.hasQualifiedName("android.content", "SharedPreferences$Editor") }
37+
}
38+
39+
/**
40+
* A method that updates a key-value pair in a
41+
* `android.content.SharedPreferences` through a `SharedPreferences.Editor`. The
42+
* value is not written until a `StorePreferenceMethod` is called.
43+
*/
44+
class PutSharedPreferenceMethod extends Method {
45+
PutSharedPreferenceMethod() {
46+
this.getDeclaringType() instanceof SharedPreferencesEditor and
47+
this.getName().matches("put%")
48+
}
49+
}
50+
51+
/** A method on `SharedPreferences.Editor` that writes the pending changes to the underlying `android.content.SharedPreferences`. */
52+
class StoreSharedPreferenceMethod extends Method {
53+
StoreSharedPreferenceMethod() {
54+
this.getDeclaringType() instanceof SharedPreferencesEditor and
55+
this.hasName(["commit", "apply"])
56+
}
57+
}

‎java/ql/test/experimental/query-tests/security/CWE-312/CleartextStorageSharedPrefs.expected‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
| CleartextStorageSharedPrefs.java:19:3:19:17 | commit(...) | 'SharedPreferences' class $@ containing $@ is stored here. Data was added $@. | CleartextStorageSharedPrefs.java:16:19:16:36 | edit(...) | edit(...) | CleartextStorageSharedPrefs.java:18:32:18:39 | password | sensitive data | CleartextStorageSharedPrefs.java:18:32:18:39 | password | here |

0 commit comments

Comments
 (0)