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

Enhance the query and add more test cases

Browse Source
This commit is contained in:
luchua-bc
2020-11-25 04:33:26 +00:00
parent a311462791
commit a49160423b
5 changed files with 175 additions and 66 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
java/ql/src/Security/CWE/CWE-312/ClearTextStorageSharedPrefs.qhelp
View File

@@ -2,7 +2,7 @@
<qhelp>
<overview>
<p>
<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 <code>SharedPreferences</code>. However, sensitive information shall 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.
<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.
</p>
</overview>

65
java/ql/src/Security/CWE/CWE-312/SensitiveStorage.qll
View File

@@ -5,6 +5,7 @@ import semmle.code.java.frameworks.android.SharedPreferences
import semmle.code.java.dataflow.TaintTracking
import semmle.code.java.dataflow.DataFlow3
import semmle.code.java.dataflow.DataFlow4
import semmle.code.java.dataflow.DataFlow5
import semmle.code.java.security.SensitiveActions
/** Test code filter. */
@@ -30,7 +31,8 @@ private class SensitiveSourceFlowConfig extends TaintTracking::Configuration {
)
or
exists(MethodAccess m |
m.getMethod() instanceof SharedPreferencesSetMethod and sink.asExpr() = m.getArgument(1)
m.getMethod() instanceof SharedPreferences::SharedPreferencesSetMethod and
sink.asExpr() = m.getArgument(1)
)
or
sink.asExpr() = getInstanceInput(_, _)
@@ -252,8 +254,9 @@ class Marshallable extends ClassStore {
/* Holds if the method call is a setter method of `SharedPreferences`. */
private predicate sharedPreferencesInput(DataFlow::Node sharedPrefs, Expr input) {
exists(MethodAccess m |
m.getMethod() instanceof SharedPreferencesSetMethod and
m.getMethod() instanceof SharedPreferences::SharedPreferencesSetMethod and
input = m.getArgument(1) and
not exists(EncryptedValueFlowConfig conf | conf.hasFlow(_, DataFlow::exprNode(input))) and
sharedPrefs.asExpr() = m.getQualifier()
)
}
@@ -261,13 +264,13 @@ private predicate sharedPreferencesInput(DataFlow::Node sharedPrefs, Expr input)
/* Holds if the method call is the store method of `SharedPreferences`. */
private predicate sharedPreferencesStore(DataFlow::Node sharedPrefs, Expr store) {
exists(MethodAccess m |
m.getMethod() instanceof SharedPreferencesStoreMethod and
m.getMethod() instanceof SharedPreferences::SharedPreferencesStoreMethod and
store = m and
sharedPrefs.asExpr() = m.getQualifier()
)
}
/* Flow from `SharedPreferences` to the method call changing its value. */
/* Flow from `SharedPreferences` to either a setter or a store method. */
class SharedPreferencesFlowConfig extends TaintTracking::Configuration {
SharedPreferencesFlowConfig() { this = "SensitiveStorage::SharedPreferencesFlowConfig" }
@@ -279,25 +282,63 @@ class SharedPreferencesFlowConfig extends TaintTracking::Configuration {
sharedPreferencesInput(sink, _) or
sharedPreferencesStore(sink, _)
}
}
override predicate isSanitizer(DataFlow::Node n) {
/**
* Method call of encrypting sensitive information.
* 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.
*/
class EncryptedSensitiveMethodAccess extends MethodAccess {
EncryptedSensitiveMethodAccess() {
getMethod().getName().toLowerCase().matches(["%encrypt%", "%hash%"])
}
}
/* Flow configuration of encrypting sensitive information. */
class EncryptedValueFlowConfig extends DataFlow5::Configuration {
EncryptedValueFlowConfig() { this = "SensitiveStorage::EncryptedValueFlowConfig" }
override predicate isSource(DataFlow5::Node src) {
exists(EncryptedSensitiveMethodAccess ema | src.asExpr() = ema.getAnArgument())
}
override predicate isSink(DataFlow5::Node sink) {
exists(MethodAccess ma |
ma.getMethod().getName().toLowerCase().matches("%encrypt%") and
n.asExpr() = ma.getAnArgument()
ma.getMethod() instanceof SharedPreferences::SharedPreferencesSetMethod and
sink.asExpr() = ma.getArgument(1)
)
}
override predicate isAdditionalFlowStep(DataFlow5::Node n1, DataFlow5::Node n2) {
exists(EncryptedSensitiveMethodAccess ema |
n1.asExpr() = ema.getAnArgument() and
n2.asExpr() = ema
)
}
}
/* Flow from the create method of `androidx.security.crypto.EncryptedSharedPreferences` to its instance. */
private class EncryptedSharedPrefFlowConfig extends DataFlow3::Configuration {
EncryptedSharedPrefFlowConfig() { this = "SensitiveStorage::EncryptedSharedPrefFlowConfig" }
override predicate isSource(DataFlow::Node src) {
src.asExpr().(MethodAccess).getMethod() instanceof
SharedPreferences::EncryptedSharedPrefsCreateMethod
}
override predicate isSink(DataFlow::Node sink) {
sink.asExpr().getType() instanceof SharedPreferences::TypeSharedPreferences
}
}
/** The call to get a `SharedPreferences.Editor` object, which can set shared preferences or be stored to device. */
class SharedPreferencesEditor extends MethodAccess {
SharedPreferencesEditor() {
this.getMethod() instanceof SharedPreferencesGetEditorMethod and
this.getMethod() instanceof SharedPreferences::SharedPreferencesGetEditorMethod and
not exists(
MethodAccess cma // not exists `SharedPreferences sharedPreferences = EncryptedSharedPreferences.create(...)`
EncryptedSharedPrefFlowConfig config // not exists `SharedPreferences sharedPreferences = EncryptedSharedPreferences.create(...)`
|
cma.getQualifier().getType() instanceof TypeEncryptedSharedPreferences and
cma.getMethod().hasName("create") and
cma.getParent().(VariableAssign).getDestVar().getAnAccess() = this.getQualifier()
config.hasFlow(_, DataFlow::exprNode(this.getQualifier()))
)
}

106
java/ql/src/semmle/code/java/frameworks/android/SharedPreferences.qll
View File

@@ -1,52 +1,64 @@
/* Definitions related to `android.content.SharedPreferences`. */
import semmle.code.java.Type
/* The interface `android.content.SharedPreferences` */
library class TypeSharedPreferences extends Interface {
TypeSharedPreferences() { hasQualifiedName("android.content", "SharedPreferences") }
}
/* Definitions related to `android.content.SharedPreferences`. */
module SharedPreferences {
/* The interface `android.content.SharedPreferences` */
class TypeSharedPreferences extends Interface {
TypeSharedPreferences() { hasQualifiedName("android.content", "SharedPreferences") }
}
/* The class `androidx.security.crypto.EncryptedSharedPreferences`, which implements `SharedPreferences` with encryption support. */
library class TypeEncryptedSharedPreferences extends Class {
TypeEncryptedSharedPreferences() {
hasQualifiedName("androidx.security.crypto", "EncryptedSharedPreferences")
}
}
/* A getter method of `android.content.SharedPreferences`. */
library class SharedPreferencesGetMethod extends Method {
SharedPreferencesGetMethod() {
getDeclaringType() instanceof TypeSharedPreferences and
getName().matches("get%")
}
}
/* Returns `android.content.SharedPreferences.Editor` from the `edit` call of `android.content.SharedPreferences`. */
library class SharedPreferencesGetEditorMethod extends Method {
SharedPreferencesGetEditorMethod() {
getDeclaringType() instanceof TypeSharedPreferences and
hasName("edit") and
getReturnType() instanceof TypeSharedPreferencesEditor
}
}
/* Definitions related to `android.content.SharedPreferences.Editor`. */
library class TypeSharedPreferencesEditor extends Interface {
TypeSharedPreferencesEditor() { hasQualifiedName("android.content", "SharedPreferences$Editor") }
}
/* A setter method for `android.content.SharedPreferences`. */
library class SharedPreferencesSetMethod extends Method {
SharedPreferencesSetMethod() {
getDeclaringType() instanceof TypeSharedPreferencesEditor and
getName().matches("put%")
}
}
/* A setter method for `android.content.SharedPreferences`. */
library class SharedPreferencesStoreMethod extends Method {
SharedPreferencesStoreMethod() {
getDeclaringType() instanceof TypeSharedPreferencesEditor and
hasName(["commit", "apply"])
/* The class `androidx.security.crypto.EncryptedSharedPreferences`, which implements `SharedPreferences` with encryption support. */
class TypeEncryptedSharedPreferences extends Class {
TypeEncryptedSharedPreferences() {
hasQualifiedName("androidx.security.crypto", "EncryptedSharedPreferences")
}
}
/* The create method of `androidx.security.crypto.EncryptedSharedPreferences` */
class EncryptedSharedPrefsCreateMethod extends Method {
EncryptedSharedPrefsCreateMethod() {
getDeclaringType() instanceof TypeEncryptedSharedPreferences and
hasName("create")
}
}
/* A getter method of `android.content.SharedPreferences`. */
class SharedPreferencesGetMethod extends Method {
SharedPreferencesGetMethod() {
getDeclaringType() instanceof TypeSharedPreferences and
getName().matches("get%")
}
}
/* Returns `android.content.SharedPreferences.Editor` from the `edit` call of `android.content.SharedPreferences`. */
class SharedPreferencesGetEditorMethod extends Method {
SharedPreferencesGetEditorMethod() {
getDeclaringType() instanceof TypeSharedPreferences and
hasName("edit") and
getReturnType() instanceof TypeSharedPreferencesEditor
}
}
/* Definitions related to `android.content.SharedPreferences.Editor`. */
class TypeSharedPreferencesEditor extends Interface {
TypeSharedPreferencesEditor() {
hasQualifiedName("android.content", "SharedPreferences$Editor")
}
}
/* A setter method for `android.content.SharedPreferences`. */
class SharedPreferencesSetMethod extends Method {
SharedPreferencesSetMethod() {
getDeclaringType() instanceof TypeSharedPreferencesEditor and
getName().matches("put%")
}
}
/* A setter method for `android.content.SharedPreferences`. */
class SharedPreferencesStoreMethod extends Method {
SharedPreferencesStoreMethod() {
getDeclaringType() instanceof TypeSharedPreferencesEditor and
hasName(["commit", "apply"])
}
}
}

2
java/ql/test/query-tests/security/CWE-312/semmle/tests/ClearTextStorageSharedPrefs.expected
View File

@@ -1 +1 @@
| ClearTextStorageSharedPrefs.java:16:3:16:17 | commit(...) | 'SharedPreferences' class $@ containing $@ is stored here. Data was added $@. | ClearTextStorageSharedPrefs.java:13:19:13:36 | edit(...) | edit(...) | ClearTextStorageSharedPrefs.java:15:32:15:39 | password | sensitive data | ClearTextStorageSharedPrefs.java:15:32:15:39 | password | here |
| 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 |

66
java/ql/test/query-tests/security/CWE-312/semmle/tests/ClearTextStorageSharedPrefs.java
View File

@@ -4,8 +4,11 @@ import android.content.SharedPreferences;
import android.content.SharedPreferences.Editor;
import androidx.security.crypto.MasterKey;
import androidx.security.crypto.EncryptedSharedPreferences;
import java.nio.charset.StandardCharsets;
import java.util.Base64;
import java.security.MessageDigest;
/** Android activity that tests saving sensitive information in `SharedPreferences` */
/* Android activity that tests saving sensitive information in `SharedPreferences` */
public class ClearTextStorageSharedPrefs extends Activity {
// BAD - save sensitive information in cleartext
public void testSetSharedPrefs1(Context context, String name, String password) {
@@ -26,13 +29,27 @@ public class ClearTextStorageSharedPrefs extends Activity {
}
private static String encrypt(String cleartext) {
//Use an encryption or hashing algorithm in real world. The demo below just returns an arbitrary value.
String cipher = "whatever_encrypted";
return cipher;
// Use an encryption or hashing algorithm in real world. The demo below just returns its hash.
MessageDigest digest = MessageDigest.getInstance("SHA-256");
byte[] hash = digest.digest(cleartext.getBytes(StandardCharsets.UTF_8));
String encoded = Base64.getEncoder().encodeToString(hash);
return encoded;
}
// GOOD - save sensitive information using the built-in `EncryptedSharedPreferences` class in androidx.
// GOOD - save sensitive information in encrypted format using separate variables
public void testSetSharedPrefs3(Context context, String name, String password) {
String encUsername = encrypt(name);
String encPassword = encrypt(password);
SharedPreferences sharedPrefs = context.getSharedPreferences("user_prefs", Context.MODE_PRIVATE);
Editor editor = sharedPrefs.edit();
editor.putString("name", encUsername);
editor.putString("password", encPassword);
editor.commit();
}
// GOOD - save sensitive information using the built-in `EncryptedSharedPreferences` class in androidx
public void testSetSharedPrefs4(Context context, String name, String password) {
MasterKey masterKey = new MasterKey.Builder(context, MasterKey.DEFAULT_MASTER_KEY_ALIAS)
.setKeyScheme(MasterKey.KeyScheme.AES256_GCM)
.build();
@@ -50,4 +67,43 @@ public class ClearTextStorageSharedPrefs extends Activity {
editor.putString("password", password);
editor.commit();
}
// GOOD - save sensitive information using the built-in `EncryptedSharedPreferences` class in androidx
public void testSetSharedPrefs5(Context context, String name, String password) {
MasterKey masterKey = new MasterKey.Builder(context, MasterKey.DEFAULT_MASTER_KEY_ALIAS)
.setKeyScheme(MasterKey.KeyScheme.AES256_GCM)
.build();
SharedPreferences.Editor editor = EncryptedSharedPreferences.create(
context,
"secret_shared_prefs",
masterKey,
EncryptedSharedPreferences.PrefKeyEncryptionScheme.AES256_SIV,
EncryptedSharedPreferences.PrefValueEncryptionScheme.AES256_GCM)
.edit();
// Use the shared preferences and editor as you normally would
editor.putString("name", name);
editor.putString("password", password);
editor.commit();
}
// GOOD - save sensitive information using the built-in `EncryptedSharedPreferences` class in androidx
public void testSetSharedPrefs6(Context context, String name, String password) {
MasterKey masterKey = new MasterKey.Builder(context, MasterKey.DEFAULT_MASTER_KEY_ALIAS)
.setKeyScheme(MasterKey.KeyScheme.AES256_GCM)
.build();
SharedPreferences.Editor editor = EncryptedSharedPreferences.create(
context,
"secret_shared_prefs",
masterKey,
EncryptedSharedPreferences.PrefKeyEncryptionScheme.AES256_SIV,
EncryptedSharedPreferences.PrefValueEncryptionScheme.AES256_GCM)
.edit()
.putString("name", name) // Use the shared preferences and editor as you normally would
.putString("password", password);
editor.commit();
}
}