Skip to content

Commit a031b2a

Browse files
aschackmullaschackmull
authored
Merge pull request #6493 from atorralba/atorralba/cleartext-storage-query-refactor
Java: Refactor Cleartext Storage queries
2 parents 6be4b3b + b52a2cd commit a031b2a

File tree

8 files changed

+322
-260
lines changed

8 files changed

+322
-260
lines changed

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

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
/** Provides classes and predicates to reason about cleartext storage in serializable classes. */
2+
3+
import java
4+
import semmle.code.java.frameworks.JAXB
5+
import semmle.code.java.dataflow.DataFlow
6+
import semmle.code.java.dataflow.DataFlow2
7+
import semmle.code.java.security.CleartextStorageQuery
8+
import semmle.code.java.security.CleartextStoragePropertiesQuery
9+
10+
private class ClassCleartextStorageSink extends CleartextStorageSink {
11+
ClassCleartextStorageSink() { this.asExpr() = getInstanceInput(_, _) }
12+
}
13+
14+
/** The instantiation of a storable class, which can be stored to disk. */
15+
abstract class ClassStore extends Storable, ClassInstanceExpr {
16+
/** Gets an input, for example `input` in `instance.password = input`. */
17+
override Expr getAnInput() {
18+
exists(ClassStoreFlowConfig conf, DataFlow::Node instance |
19+
conf.hasFlow(DataFlow::exprNode(this), instance) and
20+
result = getInstanceInput(instance, this.getConstructor().getDeclaringType())
21+
)
22+
}
23+
}
24+
25+
/**
26+
* The instantiation of a serializable class, which can be stored to disk.
27+
*
28+
* Only includes tainted instances where data from a `SensitiveSource` may flow
29+
* to an input of the `Serializable`.
30+
*/
31+
private class Serializable extends ClassStore {
32+
Serializable() {
33+
this.getConstructor().getDeclaringType().getASupertype*() instanceof TypeSerializable and
34+
// `Properties` are `Serializable`, but handled elsewhere.
35+
not this instanceof Properties and
36+
// restrict attention to tainted instances
37+
exists(SensitiveSource data |
38+
data.flowsTo(getInstanceInput(_, this.getConstructor().getDeclaringType()))
39+
)
40+
}
41+
42+
/** Gets a store, for example `outputStream.writeObject(instance)`. */
43+
override Expr getAStore() {
44+
exists(ClassStoreFlowConfig conf, DataFlow::Node n |
45+
serializableStore(n, result) and
46+
conf.hasFlow(DataFlow::exprNode(this), n)
47+
)
48+
}
49+
}
50+
51+
/** The instantiation of a marshallable class, which can be stored to disk as XML. */
52+
private class Marshallable extends ClassStore {
53+
Marshallable() { this.getConstructor().getDeclaringType() instanceof JAXBElement }
54+
55+
/** Gets a store, for example `marshaller.marshal(instance)`. */
56+
override Expr getAStore() {
57+
exists(ClassStoreFlowConfig conf, DataFlow::Node n |
58+
marshallableStore(n, result) and
59+
conf.hasFlow(DataFlow::exprNode(this), n)
60+
)
61+
}
62+
}
63+
64+
/** Gets an input, for example `input` in `instance.password = input`. */
65+
private Expr getInstanceInput(DataFlow::Node instance, RefType t) {
66+
exists(AssignExpr a, FieldAccess fa |
67+
instance = DataFlow::getFieldQualifier(fa) and
68+
a.getDest() = fa and
69+
a.getSource() = result and
70+
fa.getField().getDeclaringType() = t
71+
|
72+
t.getASourceSupertype*() instanceof TypeSerializable or
73+
t instanceof JAXBElement
74+
)
75+
}
76+
77+
private class ClassStoreFlowConfig extends DataFlow2::Configuration {
78+
ClassStoreFlowConfig() { this = "ClassStoreFlowConfig" }
79+
80+
override predicate isSource(DataFlow::Node src) { src.asExpr() instanceof ClassStore }
81+
82+
override predicate isSink(DataFlow::Node sink) {
83+
exists(getInstanceInput(sink, _)) or
84+
serializableStore(sink, _) or
85+
marshallableStore(sink, _)
86+
}
87+
88+
override int fieldFlowBranchLimit() { result = 1 }
89+
}
90+
91+
private predicate serializableStore(DataFlow::Node instance, Expr store) {
92+
exists(MethodAccess m |
93+
store = m and
94+
m.getMethod() instanceof WriteObjectMethod and
95+
instance.asExpr() = m.getArgument(0)
96+
)
97+
}
98+
99+
private predicate marshallableStore(DataFlow::Node instance, Expr store) {
100+
exists(MethodAccess m |
101+
store = m and
102+
m.getMethod() instanceof JAXBMarshalMethod and
103+
instance.asExpr() = m.getArgument(0)
104+
)
105+
}

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

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
/** Provides classes and predicates to reason about cleartext storage in cookies. */
2+
3+
import java
4+
import semmle.code.java.dataflow.DataFlow
5+
import semmle.code.java.dataflow.DataFlow3
6+
import semmle.code.java.security.CleartextStorageQuery
7+
8+
private class CookieCleartextStorageSink extends CleartextStorageSink {
9+
CookieCleartextStorageSink() { this.asExpr() = cookieInput(_) }
10+
}
11+
12+
/** The instantiation of a cookie, which can act as storage. */
13+
class Cookie extends Storable, ClassInstanceExpr {
14+
Cookie() {
15+
this.getConstructor().getDeclaringType().hasQualifiedName("javax.servlet.http", "Cookie")
16+
}
17+
18+
/** Gets an input, for example `input` in `new Cookie("...", input);`. */
19+
override Expr getAnInput() { result = cookieInput(this) }
20+
21+
/** Gets a store, for example `response.addCookie(cookie);`. */
22+
override Expr getAStore() {
23+
exists(CookieToStoreFlowConfig conf, DataFlow::Node n |
24+
cookieStore(n, result) and
25+
conf.hasFlow(DataFlow::exprNode(this), n)
26+
)
27+
}
28+
}
29+
30+
private predicate cookieStore(DataFlow::Node cookie, Expr store) {
31+
exists(MethodAccess m, Method def |
32+
m.getMethod() = def and
33+
def.getName() = "addCookie" and
34+
def.getDeclaringType().hasQualifiedName("javax.servlet.http", "HttpServletResponse") and
35+
store = m and
36+
cookie.asExpr() = m.getAnArgument()
37+
)
38+
}
39+
40+
private class CookieToStoreFlowConfig extends DataFlow3::Configuration {
41+
CookieToStoreFlowConfig() { this = "CookieToStoreFlowConfig" }
42+
43+
override predicate isSource(DataFlow::Node src) { src.asExpr() instanceof Cookie }
44+
45+
override predicate isSink(DataFlow::Node sink) { cookieStore(sink, _) }
46+
}
47+
48+
private Expr cookieInput(Cookie c) { result = c.getArgument(1) }

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

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
/** Provides classes and predicates to reason about cleartext storage in Properties files. */
2+
3+
import java
4+
import semmle.code.java.dataflow.DataFlow
5+
import semmle.code.java.frameworks.Properties
6+
import semmle.code.java.security.CleartextStorageQuery
7+
8+
private class PropertiesCleartextStorageSink extends CleartextStorageSink {
9+
PropertiesCleartextStorageSink() {
10+
exists(MethodAccess m |
11+
m.getMethod() instanceof PropertiesSetPropertyMethod and this.asExpr() = m.getArgument(1)
12+
)
13+
}
14+
}
15+
16+
/** The instantiation of a `Properties` object, which can be stored to disk. */
17+
class Properties extends Storable, ClassInstanceExpr {
18+
Properties() { this.getConstructor().getDeclaringType() instanceof TypeProperty }
19+
20+
/** Gets an input, for example `input` in `props.setProperty("password", input);`. */
21+
override Expr getAnInput() {
22+
exists(PropertiesFlowConfig conf, DataFlow::Node n |
23+
propertiesInput(n, result) and
24+
conf.hasFlow(DataFlow::exprNode(this), n)
25+
)
26+
}
27+
28+
/** Gets a store, for example `props.store(outputStream, "...")`. */
29+
override Expr getAStore() {
30+
exists(PropertiesFlowConfig conf, DataFlow::Node n |
31+
propertiesStore(n, result) and
32+
conf.hasFlow(DataFlow::exprNode(this), n)
33+
)
34+
}
35+
}
36+
37+
private predicate propertiesInput(DataFlow::Node prop, Expr input) {
38+
exists(MethodAccess m |
39+
m.getMethod() instanceof PropertiesSetPropertyMethod and
40+
input = m.getArgument(1) and
41+
prop.asExpr() = m.getQualifier()
42+
)
43+
}
44+
45+
private predicate propertiesStore(DataFlow::Node prop, Expr store) {
46+
exists(MethodAccess m |
47+
m.getMethod() instanceof PropertiesStoreMethod and
48+
store = m and
49+
prop.asExpr() = m.getQualifier()
50+
)
51+
}
52+
53+
private class PropertiesFlowConfig extends DataFlow::Configuration {
54+
PropertiesFlowConfig() { this = "PropertiesFlowConfig" }
55+
56+
override predicate isSource(DataFlow::Node src) { src.asExpr() instanceof Properties }
57+
58+
override predicate isSink(DataFlow::Node sink) {
59+
propertiesInput(sink, _) or
60+
propertiesStore(sink, _)
61+
}
62+
}

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

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
/** Provides classes and predicates to reason about cleartext storage vulnerabilities. */
2+
3+
import java
4+
private import semmle.code.java.dataflow.DataFlow4
5+
private import semmle.code.java.dataflow.TaintTracking
6+
private import semmle.code.java.security.SensitiveActions
7+
8+
/** A sink representing persistent storage that saves data in clear text. */
9+
abstract class CleartextStorageSink extends DataFlow::Node { }
10+
11+
/** A sanitizer for flows tracking sensitive data being stored in persistent storage. */
12+
abstract class CleartextStorageSanitizer extends DataFlow::Node { }
13+
14+
/** An additional taint step for sensitive data flowing into cleartext storage. */
15+
class CleartextStorageAdditionalTaintStep extends Unit {
16+
abstract predicate step(DataFlow::Node n1, DataFlow::Node n2);
17+
}
18+
19+
/** Class for expressions that may represent 'sensitive' information */
20+
class SensitiveSource extends Expr instanceof SensitiveExpr {
21+
/** Holds if this source flows to the `sink`. */
22+
predicate flowsTo(Expr sink) {
23+
exists(SensitiveSourceFlowConfig conf |
24+
conf.hasFlow(DataFlow::exprNode(this), DataFlow::exprNode(sink))
25+
)
26+
}
27+
}
28+
29+
/**
30+
* Class representing entities that may be stored/written, with methods
31+
* for finding values that are stored within them, and cases
32+
* of the entity being stored.
33+
*/
34+
abstract class Storable extends Call {
35+
/** Gets an "input" that is stored in an instance of this class. */
36+
abstract Expr getAnInput();
37+
38+
/** Gets an expression where an instance of this class is stored (e.g. to disk). */
39+
abstract Expr getAStore();
40+
}
41+
42+
private class SensitiveSourceFlowConfig extends TaintTracking::Configuration {
43+
SensitiveSourceFlowConfig() { this = "SensitiveSourceFlowConfig" }
44+
45+
override predicate isSource(DataFlow::Node src) { src.asExpr() instanceof SensitiveExpr }
46+
47+
override predicate isSink(DataFlow::Node sink) { sink instanceof CleartextStorageSink }
48+
49+
override predicate isSanitizer(DataFlow::Node sanitizer) {
50+
sanitizer instanceof CleartextStorageSanitizer
51+
}
52+
53+
override predicate isAdditionalTaintStep(DataFlow::Node n1, DataFlow::Node n2) {
54+
any(CleartextStorageAdditionalTaintStep c).step(n1, n2)
55+
}
56+
}
57+
58+
private class DefaultCleartextStorageSanitizer extends CleartextStorageSanitizer {
59+
DefaultCleartextStorageSanitizer() {
60+
this.getType() instanceof NumericType or
61+
this.getType() instanceof BooleanType or
62+
exists(EncryptedValueFlowConfig conf | conf.hasFlow(_, this))
63+
}
64+
}
65+
66+
/**
67+
* Method call for encrypting sensitive information. As there are various implementations of
68+
* encryption (reversible and non-reversible) from both JDK and third parties, this class simply
69+
* checks method name to take a best guess to reduce false positives.
70+
*/
71+
private class EncryptedSensitiveMethodAccess extends MethodAccess {
72+
EncryptedSensitiveMethodAccess() {
73+
this.getMethod().getName().toLowerCase().matches(["%encrypt%", "%hash%", "%digest%"])
74+
}
75+
}
76+
77+
/** Flow configuration for encryption methods flowing to inputs of persistent storage. */
78+
private class EncryptedValueFlowConfig extends DataFlow4::Configuration {
79+
EncryptedValueFlowConfig() { this = "EncryptedValueFlowConfig" }
80+
81+
override predicate isSource(DataFlow::Node src) {
82+
src.asExpr() instanceof EncryptedSensitiveMethodAccess
83+
}
84+
85+
override predicate isSink(DataFlow::Node sink) { sink.asExpr() instanceof SensitiveExpr }
86+
}
87+
88+
/** A taint step for `EditText.toString` in Android. */
89+
private class AndroidEditTextCleartextStorageStep extends CleartextStorageAdditionalTaintStep {
90+
override predicate step(DataFlow::Node n1, DataFlow::Node n2) {
91+
// EditText.getText() return type is parsed as `Object`, so we need to
92+
// add a taint step for `Object.toString` to model `editText.getText().toString()`
93+
exists(MethodAccess ma, Method m |
94+
ma.getMethod() = m and
95+
m.getDeclaringType() instanceof TypeObject and
96+
m.hasName("toString")
97+
|
98+
n1.asExpr() = ma.getQualifier() and n2.asExpr() = ma
99+
)
100+
}
101+
}

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

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,15 +12,12 @@
1212
*/
1313

1414
import java
15-
import SensitiveStorage
15+
import semmle.code.java.security.CleartextStorageClassQuery
1616

1717
from SensitiveSource data, ClassStore s, Expr input, Expr store
1818
where
1919
input = s.getAnInput() and
2020
store = s.getAStore() and
21-
data.flowsToCached(input) and
22-
// Exclude results in test code.
23-
not testMethod(store.getEnclosingCallable()) and
24-
not testMethod(data.getEnclosingCallable())
21+
data.flowsTo(input)
2522
select store, "Storable class $@ containing $@ is stored here. Data was added $@.", s, s.toString(),
2623
data, "sensitive data", input, "here"

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

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,15 +11,12 @@
1111
*/
1212

1313
import java
14-
import SensitiveStorage
14+
import semmle.code.java.security.CleartextStorageCookieQuery
1515

1616
from SensitiveSource data, Cookie s, Expr input, Expr store
1717
where
1818
input = s.getAnInput() and
1919
store = s.getAStore() and
20-
data.flowsToCached(input) and
21-
// Exclude results in test code.
22-
not testMethod(store.getEnclosingCallable()) and
23-
not testMethod(data.getEnclosingCallable())
20+
data.flowsTo(input)
2421
select store, "Cookie $@ containing $@ is stored here. Data was added $@.", s, s.toString(), data,
2522
"sensitive data", input, "here"

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

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,15 +11,12 @@
1111
*/
1212

1313
import java
14-
import SensitiveStorage
14+
import semmle.code.java.security.CleartextStoragePropertiesQuery
1515

1616
from SensitiveSource data, Properties s, Expr input, Expr store
1717
where
1818
input = s.getAnInput() and
1919
store = s.getAStore() and
20-
data.flowsToCached(input) and
21-
// Exclude results in test code.
22-
not testMethod(store.getEnclosingCallable()) and
23-
not testMethod(data.getEnclosingCallable())
20+
data.flowsTo(input)
2421
select store, "'Properties' class $@ containing $@ is stored here. Data was added $@.", s,
2522
s.toString(), data, "sensitive data", input, "here"

0 commit comments

Comments
 (0)