Skip to content

Commit aa79341

Browse files
committed
Refactor CleartextStorage libraries
1 parent b4130e6 commit aa79341

7 files changed

+71
-77
lines changed

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

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -29,16 +29,16 @@ class LocalDatabaseOpenMethodAccess extends Storable, Call {
2929
}
3030

3131
override Expr getAnInput() {
32-
exists(LocalDatabaseFlowConfig config, DataFlow::Node database |
32+
exists(DataFlow::Node database |
3333
localDatabaseInput(database, result) and
34-
config.hasFlow(DataFlow::exprNode(this), database)
34+
LocalDatabaseFlow::flow(DataFlow::exprNode(this), database)
3535
)
3636
}
3737

3838
override Expr getAStore() {
39-
exists(LocalDatabaseFlowConfig config, DataFlow::Node database |
39+
exists(DataFlow::Node database |
4040
localDatabaseStore(database, result) and
41-
config.hasFlow(DataFlow::exprNode(this), database)
41+
LocalDatabaseFlow::flow(DataFlow::exprNode(this), database)
4242
)
4343
}
4444
}
@@ -93,19 +93,17 @@ private predicate localDatabaseStore(DataFlow::Node database, MethodAccess store
9393
)
9494
}
9595

96-
private class LocalDatabaseFlowConfig extends DataFlow::Configuration {
97-
LocalDatabaseFlowConfig() { this = "LocalDatabaseFlowConfig" }
98-
99-
override predicate isSource(DataFlow::Node source) {
96+
private module LocalDatabaseFlowConfig implements DataFlow::ConfigSig {
97+
predicate isSource(DataFlow::Node source) {
10098
source.asExpr() instanceof LocalDatabaseOpenMethodAccess
10199
}
102100

103-
override predicate isSink(DataFlow::Node sink) {
101+
predicate isSink(DataFlow::Node sink) {
104102
localDatabaseInput(sink, _) or
105103
localDatabaseStore(sink, _)
106104
}
107105

108-
override predicate isAdditionalFlowStep(DataFlow::Node n1, DataFlow::Node n2) {
106+
predicate isAdditionalFlowStep(DataFlow::Node n1, DataFlow::Node n2) {
109107
// Adds a step for tracking databases through field flow, that is, a database is opened and
110108
// assigned to a field, and then an input or store method is called on that field elsewhere.
111109
exists(Field f |
@@ -115,3 +113,5 @@ private class LocalDatabaseFlowConfig extends DataFlow::Configuration {
115113
)
116114
}
117115
}
116+
117+
private module LocalDatabaseFlow = DataFlow::Global<LocalDatabaseFlowConfig>;

java/ql/lib/semmle/code/java/security/CleartextStorageAndroidFilesystemQuery.qll

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -24,16 +24,16 @@ class LocalFileOpenCall extends Storable {
2424
}
2525

2626
override Expr getAnInput() {
27-
exists(FilesystemFlowConfig conf, DataFlow::Node n |
27+
exists(DataFlow::Node n |
2828
filesystemInput(n, result) and
29-
conf.hasFlow(DataFlow::exprNode(this), n)
29+
FilesystemFlow::flow(DataFlow::exprNode(this), n)
3030
)
3131
}
3232

3333
override Expr getAStore() {
34-
exists(FilesystemFlowConfig conf, DataFlow::Node n |
34+
exists(DataFlow::Node n |
3535
closesFile(n, result) and
36-
conf.hasFlow(DataFlow::exprNode(this), n)
36+
FilesystemFlow::flow(DataFlow::exprNode(this), n)
3737
)
3838
}
3939
}
@@ -79,17 +79,15 @@ private class CloseFileMethod extends Method {
7979
}
8080
}
8181

82-
private class FilesystemFlowConfig extends DataFlow::Configuration {
83-
FilesystemFlowConfig() { this = "FilesystemFlowConfig" }
82+
private module FilesystemFlowConfig implements DataFlow::ConfigSig {
83+
predicate isSource(DataFlow::Node src) { src.asExpr() instanceof LocalFileOpenCall }
8484

85-
override predicate isSource(DataFlow::Node src) { src.asExpr() instanceof LocalFileOpenCall }
86-
87-
override predicate isSink(DataFlow::Node sink) {
85+
predicate isSink(DataFlow::Node sink) {
8886
filesystemInput(sink, _) or
8987
closesFile(sink, _)
9088
}
9189

92-
override predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
90+
predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
9391
// Add nested Writer constructors as extra data flow steps
9492
exists(ClassInstanceExpr cie |
9593
cie.getConstructedType().getAnAncestor().hasQualifiedName("java.io", "Writer") and
@@ -98,3 +96,5 @@ private class FilesystemFlowConfig extends DataFlow::Configuration {
9896
)
9997
}
10098
}
99+
100+
private module FilesystemFlow = DataFlow::Global<FilesystemFlowConfig>;

java/ql/lib/semmle/code/java/security/CleartextStorageClassQuery.qll

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,8 @@ private class ClassCleartextStorageSink extends CleartextStorageSink {
1414
abstract class ClassStore extends Storable, ClassInstanceExpr {
1515
/** Gets an input, for example `input` in `instance.password = input`. */
1616
override Expr getAnInput() {
17-
exists(ClassStoreFlowConfig conf, DataFlow::Node instance |
18-
conf.hasFlow(DataFlow::exprNode(this), instance) and
17+
exists(DataFlow::Node instance |
18+
ClassStoreFlow::flow(DataFlow::exprNode(this), instance) and
1919
result = getInstanceInput(instance, this.getConstructor().getDeclaringType())
2020
)
2121
}
@@ -40,9 +40,9 @@ private class Serializable extends ClassStore {
4040

4141
/** Gets a store, for example `outputStream.writeObject(instance)`. */
4242
override Expr getAStore() {
43-
exists(ClassStoreFlowConfig conf, DataFlow::Node n |
43+
exists(DataFlow::Node n |
4444
serializableStore(n, result) and
45-
conf.hasFlow(DataFlow::exprNode(this), n)
45+
ClassStoreFlow::flow(DataFlow::exprNode(this), n)
4646
)
4747
}
4848
}
@@ -53,9 +53,9 @@ private class Marshallable extends ClassStore {
5353

5454
/** Gets a store, for example `marshaller.marshal(instance)`. */
5555
override Expr getAStore() {
56-
exists(ClassStoreFlowConfig conf, DataFlow::Node n |
56+
exists(DataFlow::Node n |
5757
marshallableStore(n, result) and
58-
conf.hasFlow(DataFlow::exprNode(this), n)
58+
ClassStoreFlow::flow(DataFlow::exprNode(this), n)
5959
)
6060
}
6161
}
@@ -73,20 +73,20 @@ private Expr getInstanceInput(DataFlow::Node instance, RefType t) {
7373
)
7474
}
7575

76-
private class ClassStoreFlowConfig extends DataFlow::Configuration {
77-
ClassStoreFlowConfig() { this = "ClassStoreFlowConfig" }
76+
private module ClassStoreFlowConfig implements DataFlow::ConfigSig {
77+
predicate isSource(DataFlow::Node src) { src.asExpr() instanceof ClassStore }
7878

79-
override predicate isSource(DataFlow::Node src) { src.asExpr() instanceof ClassStore }
80-
81-
override predicate isSink(DataFlow::Node sink) {
79+
predicate isSink(DataFlow::Node sink) {
8280
exists(getInstanceInput(sink, _)) or
8381
serializableStore(sink, _) or
8482
marshallableStore(sink, _)
8583
}
8684

87-
override int fieldFlowBranchLimit() { result = 1 }
85+
int fieldFlowBranchLimit() { result = 1 }
8886
}
8987

88+
private module ClassStoreFlow = DataFlow::Global<ClassStoreFlowConfig>;
89+
9090
private predicate serializableStore(DataFlow::Node instance, Expr store) {
9191
exists(MethodAccess m |
9292
store = m and

java/ql/lib/semmle/code/java/security/CleartextStorageCookieQuery.qll

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,9 @@ class Cookie extends Storable, ClassInstanceExpr {
2020

2121
/** Gets a store, for example `response.addCookie(cookie);`. */
2222
override Expr getAStore() {
23-
exists(CookieToStoreFlowConfig conf, DataFlow::Node n |
23+
exists(DataFlow::Node n |
2424
cookieStore(n, result) and
25-
conf.hasFlow(DataFlow::exprNode(this), n)
25+
CookieToStoreFlow::flow(DataFlow::exprNode(this), n)
2626
)
2727
}
2828
}
@@ -37,12 +37,12 @@ private predicate cookieStore(DataFlow::Node cookie, Expr store) {
3737
)
3838
}
3939

40-
private class CookieToStoreFlowConfig extends DataFlow3::Configuration {
41-
CookieToStoreFlowConfig() { this = "CookieToStoreFlowConfig" }
40+
private module CookieToStoreFlowConfig implements DataFlow::ConfigSig {
41+
predicate isSource(DataFlow::Node src) { src.asExpr() instanceof Cookie }
4242

43-
override predicate isSource(DataFlow::Node src) { src.asExpr() instanceof Cookie }
44-
45-
override predicate isSink(DataFlow::Node sink) { cookieStore(sink, _) }
43+
predicate isSink(DataFlow::Node sink) { cookieStore(sink, _) }
4644
}
4745

46+
private module CookieToStoreFlow = DataFlow::Global<CookieToStoreFlowConfig>;
47+
4848
private Expr cookieInput(Cookie c) { result = c.getArgument(1) }

java/ql/lib/semmle/code/java/security/CleartextStoragePropertiesQuery.qll

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -19,17 +19,17 @@ class Properties extends Storable, ClassInstanceExpr {
1919

2020
/** Gets an input, for example `input` in `props.setProperty("password", input);`. */
2121
override Expr getAnInput() {
22-
exists(PropertiesFlowConfig conf, DataFlow::Node n |
22+
exists(DataFlow::Node n |
2323
propertiesInput(n, result) and
24-
conf.hasFlow(DataFlow::exprNode(this), n)
24+
PropertiesFlow::flow(DataFlow::exprNode(this), n)
2525
)
2626
}
2727

2828
/** Gets a store, for example `props.store(outputStream, "...")`. */
2929
override Expr getAStore() {
30-
exists(PropertiesFlowConfig conf, DataFlow::Node n |
30+
exists(DataFlow::Node n |
3131
propertiesStore(n, result) and
32-
conf.hasFlow(DataFlow::exprNode(this), n)
32+
PropertiesFlow::flow(DataFlow::exprNode(this), n)
3333
)
3434
}
3535
}
@@ -50,13 +50,13 @@ private predicate propertiesStore(DataFlow::Node prop, Expr store) {
5050
)
5151
}
5252

53-
private class PropertiesFlowConfig extends DataFlow::Configuration {
54-
PropertiesFlowConfig() { this = "PropertiesFlowConfig" }
53+
private module PropertiesFlowConfig implements DataFlow::ConfigSig {
54+
predicate isSource(DataFlow::Node src) { src.asExpr() instanceof Properties }
5555

56-
override predicate isSource(DataFlow::Node src) { src.asExpr() instanceof Properties }
57-
58-
override predicate isSink(DataFlow::Node sink) {
56+
predicate isSink(DataFlow::Node sink) {
5957
propertiesInput(sink, _) or
6058
propertiesStore(sink, _)
6159
}
6260
}
61+
62+
private module PropertiesFlow = DataFlow::Global<PropertiesFlowConfig>;

java/ql/lib/semmle/code/java/security/CleartextStorageQuery.qll

Lines changed: 14 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,7 @@ class CleartextStorageAdditionalTaintStep extends Unit {
2121
class SensitiveSource extends Expr instanceof SensitiveExpr {
2222
/** Holds if this source flows to the `sink`. */
2323
predicate flowsTo(Expr sink) {
24-
exists(SensitiveSourceFlowConfig conf |
25-
conf.hasFlow(DataFlow::exprNode(this), DataFlow::exprNode(sink))
26-
)
24+
SensitiveSourceFlow::flow(DataFlow::exprNode(this), DataFlow::exprNode(sink))
2725
}
2826
}
2927

@@ -40,27 +38,25 @@ abstract class Storable extends Call {
4038
abstract Expr getAStore();
4139
}
4240

43-
private class SensitiveSourceFlowConfig extends TaintTracking2::Configuration {
44-
SensitiveSourceFlowConfig() { this = "SensitiveSourceFlowConfig" }
41+
private module SensitiveSourceFlowConfig implements DataFlow::ConfigSig {
42+
predicate isSource(DataFlow::Node src) { src.asExpr() instanceof SensitiveExpr }
4543

46-
override predicate isSource(DataFlow::Node src) { src.asExpr() instanceof SensitiveExpr }
44+
predicate isSink(DataFlow::Node sink) { sink instanceof CleartextStorageSink }
4745

48-
override predicate isSink(DataFlow::Node sink) { sink instanceof CleartextStorageSink }
46+
predicate isBarrier(DataFlow::Node sanitizer) { sanitizer instanceof CleartextStorageSanitizer }
4947

50-
override predicate isSanitizer(DataFlow::Node sanitizer) {
51-
sanitizer instanceof CleartextStorageSanitizer
52-
}
53-
54-
override predicate isAdditionalTaintStep(DataFlow::Node n1, DataFlow::Node n2) {
48+
predicate isAdditionalFlowStep(DataFlow::Node n1, DataFlow::Node n2) {
5549
any(CleartextStorageAdditionalTaintStep c).step(n1, n2)
5650
}
5751
}
5852

53+
private module SensitiveSourceFlow = TaintTracking::Global<SensitiveSourceFlowConfig>;
54+
5955
private class DefaultCleartextStorageSanitizer extends CleartextStorageSanitizer {
6056
DefaultCleartextStorageSanitizer() {
6157
this.getType() instanceof NumericType or
6258
this.getType() instanceof BooleanType or
63-
exists(EncryptedValueFlowConfig conf | conf.hasFlow(_, this))
59+
EncryptedValueFlow::flow(_, this)
6460
}
6561
}
6662

@@ -76,12 +72,10 @@ private class EncryptedSensitiveMethodAccess extends MethodAccess {
7672
}
7773

7874
/** Flow configuration for encryption methods flowing to inputs of persistent storage. */
79-
private class EncryptedValueFlowConfig extends DataFlow4::Configuration {
80-
EncryptedValueFlowConfig() { this = "EncryptedValueFlowConfig" }
75+
private module EncryptedValueFlowConfig implements DataFlow::ConfigSig {
76+
predicate isSource(DataFlow::Node src) { src.asExpr() instanceof EncryptedSensitiveMethodAccess }
8177

82-
override predicate isSource(DataFlow::Node src) {
83-
src.asExpr() instanceof EncryptedSensitiveMethodAccess
84-
}
85-
86-
override predicate isSink(DataFlow::Node sink) { sink.asExpr() instanceof SensitiveExpr }
78+
predicate isSink(DataFlow::Node sink) { sink.asExpr() instanceof SensitiveExpr }
8779
}
80+
81+
private module EncryptedValueFlow = DataFlow::Global<EncryptedValueFlowConfig>;

java/ql/lib/semmle/code/java/security/CleartextStorageSharedPrefsQuery.qll

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -28,17 +28,17 @@ class SharedPreferencesEditorMethodAccess extends Storable, MethodAccess {
2828

2929
/** Gets an input, for example `password` in `editor.putString("password", password);`. */
3030
override Expr getAnInput() {
31-
exists(SharedPreferencesFlowConfig conf, DataFlow::Node editor |
31+
exists(DataFlow::Node editor |
3232
sharedPreferencesInput(editor, result) and
33-
conf.hasFlow(DataFlow::exprNode(this), editor)
33+
SharedPreferencesFlow::flow(DataFlow::exprNode(this), editor)
3434
)
3535
}
3636

3737
/** Gets a store, for example `editor.commit();`. */
3838
override Expr getAStore() {
39-
exists(SharedPreferencesFlowConfig conf, DataFlow::Node editor |
39+
exists(DataFlow::Node editor |
4040
sharedPreferencesStore(editor, result) and
41-
conf.hasFlow(DataFlow::exprNode(this), editor)
41+
SharedPreferencesFlow::flow(DataFlow::exprNode(this), editor)
4242
)
4343
}
4444
}
@@ -65,15 +65,15 @@ private predicate sharedPreferencesStore(DataFlow::Node editor, MethodAccess m)
6565
}
6666

6767
/** Flow from `SharedPreferences.Editor` to either a setter or a store method. */
68-
private class SharedPreferencesFlowConfig extends DataFlow::Configuration {
69-
SharedPreferencesFlowConfig() { this = "SharedPreferencesFlowConfig" }
70-
71-
override predicate isSource(DataFlow::Node src) {
68+
private module SharedPreferencesFlowConfig implements DataFlow::ConfigSig {
69+
predicate isSource(DataFlow::Node src) {
7270
src.asExpr() instanceof SharedPreferencesEditorMethodAccess
7371
}
7472

75-
override predicate isSink(DataFlow::Node sink) {
73+
predicate isSink(DataFlow::Node sink) {
7674
sharedPreferencesInput(sink, _) or
7775
sharedPreferencesStore(sink, _)
7876
}
7977
}
78+
79+
private module SharedPreferencesFlow = DataFlow::Global<SharedPreferencesFlowConfig>;

0 commit comments

Comments
 (0)