Skip to content

Commit dc0b06a

Browse files
intrigusintrigus
committed
Java: Factor out SecurityFlag library.
1 parent 51fdcf8 commit dc0b06a

File tree

3 files changed

+118
-121
lines changed

3 files changed

+118
-121
lines changed

‎java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.ql

Lines changed: 17 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,8 @@ import java
1313
import semmle.code.java.controlflow.Guards
1414
import semmle.code.java.dataflow.DataFlow
1515
import semmle.code.java.dataflow.FlowSources
16-
import semmle.code.java.dataflow.TaintTracking2
1716
import semmle.code.java.security.Encryption
17+
import semmle.code.java.security.SecurityFlag
1818
import DataFlow::PathGraph
1919

2020
/**
@@ -80,72 +80,30 @@ class InsecureTrustManagerConfiguration extends TaintTracking::Configuration {
8080
}
8181
}
8282

83-
bindingset[result]
84-
private string getAFlagName() {
85-
result
86-
.regexpMatch("(?i).*(secure|disable|selfCert|selfSign|validat|verif|trust|ignore|nocertificatecheck).*") and
87-
result != "equalsIgnoreCase"
88-
}
89-
9083
/**
91-
* A flag has to either be of type `String`, `boolean` or `Boolean`.
84+
* Flags suggesting a deliberately insecure `TrustManager` usage.
9285
*/
93-
private class FlagType extends Type {
94-
FlagType() {
95-
this instanceof TypeString
96-
or
97-
this instanceof BooleanType
98-
}
99-
}
100-
101-
/** Holds if `source` should is considered a flag. */
102-
private predicate isFlag(DataFlow::Node source) {
103-
exists(VarAccess v | v.getVariable().getName() = getAFlagName() |
104-
source.asExpr() = v and v.getType() instanceof FlagType
105-
)
106-
or
107-
exists(StringLiteral s | s.getRepresentedString() = getAFlagName() | source.asExpr() = s)
108-
or
109-
exists(MethodAccess ma | ma.getMethod().getName() = getAFlagName() |
110-
source.asExpr() = ma and
111-
ma.getType() instanceof FlagType
112-
)
113-
}
86+
private class InsecureTrustManagerFlag extends FlagKind {
87+
InsecureTrustManagerFlag() { this = "InsecureTrustManagerFlag" }
11488

115-
/**
116-
* Holds if there is local flow from `node1` to `node2` either due to standard data-flow steps or the
117-
* following custom flow steps:
118-
* 1. `Boolean.parseBoolean(taintedValue)` taints the return value of `parseBoolean`.
119-
* 2. A call to an `EnvReadMethod` such as `System.getProperty` where a tainted value is used as an argument.
120-
* The return value of such a method is then tainted.
121-
*/
122-
private predicate flagFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
123-
DataFlow::localFlowStep(node1, node2)
124-
or
125-
exists(MethodAccess ma | ma.getMethod() = any(EnvReadMethod m) |
126-
ma = node2.asExpr() and ma.getAnArgument() = node1.asExpr()
127-
)
128-
or
129-
exists(MethodAccess ma |
130-
ma.getMethod().hasName("parseBoolean") and
131-
ma.getMethod().getDeclaringType().hasQualifiedName("java.lang", "Boolean")
132-
|
133-
ma = node2.asExpr() and ma.getAnArgument() = node1.asExpr()
134-
)
89+
bindingset[result]
90+
override string getAFlagName() {
91+
result
92+
.regexpMatch("(?i).*(secure|disable|selfCert|selfSign|validat|verif|trust|ignore|nocertificatecheck).*") and
93+
result != "equalsIgnoreCase"
94+
}
13595
}
13696

137-
/** Gets a guard that depends on a flag. */
138-
private Guard getAGuard() {
139-
exists(DataFlow::Node source, DataFlow::Node sink |
140-
isFlag(source) and
141-
flagFlowStep*(source, sink) and
142-
sink.asExpr() = result
143-
)
97+
/** Gets a guard that represents a (likely) flag controlling an insecure `TrustManager` use. */
98+
private Guard getAnInsecureTrustManagerFlagGuard() {
99+
result = any(InsecureTrustManagerFlag flag).getAFlag().asExpr()
144100
}
145101

146-
/** Holds if `node` is guarded by a flag that suggests an intentionally insecure feature. */
102+
/** Holds if `node` is guarded by a flag that suggests an intentionally insecure use. */
147103
private predicate isNodeGuardedByFlag(DataFlow::Node node) {
148-
exists(Guard g | g.controls(node.asExpr().getBasicBlock(), _) | g = getAGuard())
104+
exists(Guard g | g.controls(node.asExpr().getBasicBlock(), _) |
105+
g = getASecurityFeatureFlagGuard() or g = getAnInsecureTrustManagerFlagGuard()
106+
)
149107
}
150108

151109
from

‎java/ql/src/Security/CWE/CWE-297/UnsafeHostnameVerification.ql

Lines changed: 17 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import semmle.code.java.controlflow.Guards
1515
import semmle.code.java.dataflow.DataFlow
1616
import semmle.code.java.dataflow.FlowSources
1717
import semmle.code.java.security.Encryption
18+
import semmle.code.java.security.SecurityFlag
1819
import DataFlow::PathGraph
1920
private import semmle.code.java.dataflow.ExternalFlow
2021

@@ -86,76 +87,30 @@ private class HostnameVerifierSink extends DataFlow::Node {
8687
HostnameVerifierSink() { sinkNode(this, "set-hostname-verifier") }
8788
}
8889

89-
bindingset[result]
90-
private string getAFlagName() {
91-
result
92-
.regexpMatch("(?i).*(secure|disable|selfCert|selfSign|validat|verif|trust|ignore|nocertificatecheck).*")
93-
}
94-
9590
/**
96-
* A flag has to either be of type `String`, `boolean` or `Boolean`.
91+
* Flags suggesting a deliberately unsafe `HostnameVerifier` usage.
9792
*/
98-
private class FlagType extends Type {
99-
FlagType() {
100-
this instanceof TypeString
101-
or
102-
this instanceof BooleanType
93+
private class UnsafeHostnameVerificationFlag extends FlagKind {
94+
UnsafeHostnameVerificationFlag() { this = "UnsafeHostnameVerificationFlag" }
95+
96+
bindingset[result]
97+
override string getAFlagName() {
98+
result
99+
.regexpMatch("(?i).*(secure|disable|selfCert|selfSign|validat|verif|trust|ignore|nocertificatecheck).*") and
100+
result != "equalsIgnoreCase"
103101
}
104102
}
105103

106-
private predicate isEqualsIgnoreCaseMethodAccess(MethodAccess ma) {
107-
ma.getMethod().hasName("equalsIgnoreCase") and
108-
ma.getMethod().getDeclaringType() instanceof TypeString
109-
}
110-
111-
/** Holds if `source` should is considered a flag. */
112-
private predicate isFlag(DataFlow::Node source) {
113-
exists(VarAccess v | v.getVariable().getName() = getAFlagName() |
114-
source.asExpr() = v and v.getType() instanceof FlagType
115-
)
116-
or
117-
exists(StringLiteral s | s.getRepresentedString() = getAFlagName() | source.asExpr() = s)
118-
or
119-
exists(MethodAccess ma | ma.getMethod().getName() = getAFlagName() |
120-
source.asExpr() = ma and
121-
ma.getType() instanceof FlagType and
122-
not isEqualsIgnoreCaseMethodAccess(ma)
123-
)
124-
}
125-
126-
/**
127-
* Holds if there is flow from `node1` to `node2` either due to local flow or due to custom flow steps:
128-
* 1. `Boolean.parseBoolean(taintedValue)` taints the return value of `parseBoolean`.
129-
* 2. A call to an `EnvReadMethod` such as `System.getProperty` where a tainted value is used as an argument.
130-
* The return value of such a method is then tainted.
131-
*/
132-
private predicate flagFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
133-
DataFlow::localFlowStep(node1, node2)
134-
or
135-
exists(MethodAccess ma | ma.getMethod() = any(EnvReadMethod m) |
136-
ma = node2.asExpr() and ma.getAnArgument() = node1.asExpr()
137-
)
138-
or
139-
exists(MethodAccess ma |
140-
ma.getMethod().hasName("parseBoolean") and
141-
ma.getMethod().getDeclaringType().hasQualifiedName("java.lang", "Boolean")
142-
|
143-
ma = node2.asExpr() and ma.getAnArgument() = node1.asExpr()
144-
)
104+
/** Gets a guard that represents a (likely) flag controlling an unsafe `HostnameVerifier` use. */
105+
private Guard getAnUnsafeHostnameVerifierFlagGuard() {
106+
result = any(UnsafeHostnameVerificationFlag flag).getAFlag().asExpr()
145107
}
146108

147-
/** Gets a guard that depends on a flag. */
148-
private Guard getAGuard() {
149-
exists(DataFlow::Node source, DataFlow::Node sink |
150-
isFlag(source) and
151-
flagFlowStep*(source, sink) and
152-
sink.asExpr() = result
153-
)
154-
}
155-
156-
/** Holds if `node` is guarded by a flag that suggests an intentionally insecure feature. */
109+
/** Holds if `node` is guarded by a flag that suggests an intentionally insecure use. */
157110
private predicate isNodeGuardedByFlag(DataFlow::Node node) {
158-
exists(Guard g | g.controls(node.asExpr().getBasicBlock(), _) | g = getAGuard())
111+
exists(Guard g | g.controls(node.asExpr().getBasicBlock(), _) |
112+
g = getASecurityFeatureFlagGuard() or g = getAnUnsafeHostnameVerifierFlagGuard()
113+
)
159114
}
160115

161116
from

‎java/ql/src/semmle/code/java/security/SecurityFlag.qll

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
/**
2+
* Provides utility predicates to spot variable names, parameter names, and string literals that suggest deliberately insecure settings.
3+
*/
4+
5+
import java
6+
import semmle.code.java.controlflow.Guards
7+
import semmle.code.java.dataflow.DataFlow
8+
import semmle.code.java.dataflow.FlowSources
9+
10+
/**
11+
* A kind of flag that may indicate security expectations regarding the code it guards.
12+
*/
13+
abstract class FlagKind extends string {
14+
bindingset[this]
15+
FlagKind() { any() }
16+
17+
/**
18+
* Returns a flag name of this type.
19+
*/
20+
bindingset[result]
21+
abstract string getAFlagName();
22+
23+
/** Gets a node representing a (likely) security flag. */
24+
DataFlow::Node getAFlag() {
25+
exists(VarAccess v | v.getVariable().getName() = getAFlagName() |
26+
result.asExpr() = v and v.getType() instanceof FlagType
27+
)
28+
or
29+
exists(StringLiteral s | s.getRepresentedString() = getAFlagName() | result.asExpr() = s)
30+
or
31+
exists(MethodAccess ma | ma.getMethod().getName() = getAFlagName() |
32+
result.asExpr() = ma and
33+
ma.getType() instanceof FlagType
34+
)
35+
or
36+
flagFlowStep*(getAFlag(), result)
37+
}
38+
}
39+
40+
/**
41+
* Flags suggesting an optional feature, perhaps deliberately insecure.
42+
*/
43+
class SecurityFeatureFlag extends FlagKind {
44+
SecurityFeatureFlag() { this = "SecurityFeatureFlag" }
45+
46+
bindingset[result]
47+
override string getAFlagName() { result.regexpMatch("(?i).*(secure|(en|dis)able).*") }
48+
}
49+
50+
/**
51+
* A flag has to either be of type `String`, `boolean` or `Boolean`.
52+
*/
53+
private class FlagType extends Type {
54+
FlagType() {
55+
this instanceof TypeString
56+
or
57+
this instanceof BooleanType
58+
}
59+
}
60+
61+
/**
62+
* Holds if there is local flow from `node1` to `node2` either due to standard data-flow steps or the
63+
* following custom flow steps:
64+
* 1. `Boolean.parseBoolean(taintedValue)` taints the return value of `parseBoolean`.
65+
* 2. A call to an `EnvReadMethod` such as `System.getProperty` where a tainted value is used as an argument.
66+
* The return value of such a method is then tainted.
67+
*/
68+
private predicate flagFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
69+
DataFlow::localFlowStep(node1, node2)
70+
or
71+
exists(MethodAccess ma | ma.getMethod() = any(EnvReadMethod m) |
72+
ma = node2.asExpr() and ma.getAnArgument() = node1.asExpr()
73+
)
74+
or
75+
exists(MethodAccess ma |
76+
ma.getMethod().hasName("parseBoolean") and
77+
ma.getMethod().getDeclaringType().hasQualifiedName("java.lang", "Boolean")
78+
|
79+
ma = node2.asExpr() and ma.getAnArgument() = node1.asExpr()
80+
)
81+
}
82+
83+
/** Gets a guard that represents a (likely) security feature-flag check. */
84+
Guard getASecurityFeatureFlagGuard() { result = any(SecurityFeatureFlag flag).getAFlag().asExpr() }

0 commit comments

Comments
 (0)