Skip to content

Commit e4775e0

Browse files
committed
Java: Remove "intention-guessing" sanitizer & simplify.
This removes the sanitizer part that classified some results as FP if the results were in methods with certain names, like `disableVerification()`. I now think that it's a bad idea to filter based on the method name. The custom flow steps in `flagFlowStep` are now listed explicitly. Simplified check whether a method throws an exception.
1 parent 8a7f6b7 commit e4775e0

File tree

1 file changed

+8
-29
lines changed

1 file changed

+8
-29
lines changed

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

Lines changed: 8 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,8 @@ private class CertificateException extends RefType {
4444
* - `m` calls a method declared to throw a `CertificateException`, but for which no source is available
4545
*/
4646
private predicate mayThrowCertificateException(Method m) {
47-
exists(Stmt stmt | m.getBody().getAChild*() = stmt |
48-
stmt.(ThrowStmt).getThrownExceptionType().getASupertype*() instanceof CertificateException
49-
)
47+
m.getBody().getAChild*().(ThrowStmt).getThrownExceptionType().getASupertype*() instanceof
48+
CertificateException
5049
or
5150
exists(Method otherMethod | m.polyCalls(otherMethod) |
5251
mayThrowCertificateException(otherMethod)
@@ -75,31 +74,6 @@ class InsecureTrustManagerConfiguration extends TaintTracking::Configuration {
7574
ma.getArgument(1) = sink.asExpr()
7675
)
7776
}
78-
79-
override predicate isSanitizer(DataFlow::Node barrier) {
80-
// ignore nodes that are in functions that intentionally trust all certificates
81-
barrier
82-
.getEnclosingCallable()
83-
.getName()
84-
/*
85-
* Regex: (_)* :
86-
* some methods have underscores.
87-
* Regex: (no|ignore|disable)(strictssl|ssl|verify|verification)
88-
* noStrictSSL ignoreSsl
89-
* Regex: (set)?(accept|trust|ignore|allow)(all|every|any|selfsigned)
90-
* acceptAll trustAll ignoreAll setTrustAnyHttps
91-
* Regex: (use|do|enable)insecure
92-
* useInsecureSSL
93-
* Regex: (set|do|use)?no.*(check|validation|verify|verification)
94-
* setNoCertificateCheck
95-
* Regex: disable
96-
* disableChecks
97-
*/
98-
99-
.regexpMatch("^(?i)(_)*((no|ignore|disable)(strictssl|ssl|verify|verification)" +
100-
"|(set)?(accept|trust|ignore|allow)(all|every|any|selfsigned)" +
101-
"|(use|do|enable)insecure|(set|do|use)?no.*(check|validation|verify|verification)|disable).*$")
102-
}
10377
}
10478

10579
bindingset[result]
@@ -139,7 +113,12 @@ private predicate isFlag(DataFlow::Node source) {
139113
)
140114
}
141115

142-
/** Holds if there is flow from `node1` to `node2` either due to local flow or due to custom flow steps. */
116+
/**
117+
* Holds if there is flow from `node1` to `node2` either due to local flow or due to 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+
*/
143122
private predicate flagFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
144123
DataFlow::localFlowStep(node1, node2)
145124
or

0 commit comments

Comments
 (0)