mirror of
https://github.com/github/codeql.git
synced 2025-12-24 04:36:35 +01:00
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.
This commit is contained in:
@@ -44,9 +44,8 @@ private class CertificateException extends RefType {
|
|||||||
* - `m` calls a method declared to throw a `CertificateException`, but for which no source is available
|
* - `m` calls a method declared to throw a `CertificateException`, but for which no source is available
|
||||||
*/
|
*/
|
||||||
private predicate mayThrowCertificateException(Method m) {
|
private predicate mayThrowCertificateException(Method m) {
|
||||||
exists(Stmt stmt | m.getBody().getAChild*() = stmt |
|
m.getBody().getAChild*().(ThrowStmt).getThrownExceptionType().getASupertype*() instanceof
|
||||||
stmt.(ThrowStmt).getThrownExceptionType().getASupertype*() instanceof CertificateException
|
CertificateException
|
||||||
)
|
|
||||||
or
|
or
|
||||||
exists(Method otherMethod | m.polyCalls(otherMethod) |
|
exists(Method otherMethod | m.polyCalls(otherMethod) |
|
||||||
mayThrowCertificateException(otherMethod)
|
mayThrowCertificateException(otherMethod)
|
||||||
@@ -75,31 +74,6 @@ class InsecureTrustManagerConfiguration extends TaintTracking::Configuration {
|
|||||||
ma.getArgument(1) = sink.asExpr()
|
ma.getArgument(1) = sink.asExpr()
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
override predicate isSanitizer(DataFlow::Node barrier) {
|
|
||||||
// ignore nodes that are in functions that intentionally trust all certificates
|
|
||||||
barrier
|
|
||||||
.getEnclosingCallable()
|
|
||||||
.getName()
|
|
||||||
/*
|
|
||||||
* Regex: (_)* :
|
|
||||||
* some methods have underscores.
|
|
||||||
* Regex: (no|ignore|disable)(strictssl|ssl|verify|verification)
|
|
||||||
* noStrictSSL ignoreSsl
|
|
||||||
* Regex: (set)?(accept|trust|ignore|allow)(all|every|any|selfsigned)
|
|
||||||
* acceptAll trustAll ignoreAll setTrustAnyHttps
|
|
||||||
* Regex: (use|do|enable)insecure
|
|
||||||
* useInsecureSSL
|
|
||||||
* Regex: (set|do|use)?no.*(check|validation|verify|verification)
|
|
||||||
* setNoCertificateCheck
|
|
||||||
* Regex: disable
|
|
||||||
* disableChecks
|
|
||||||
*/
|
|
||||||
|
|
||||||
.regexpMatch("^(?i)(_)*((no|ignore|disable)(strictssl|ssl|verify|verification)" +
|
|
||||||
"|(set)?(accept|trust|ignore|allow)(all|every|any|selfsigned)" +
|
|
||||||
"|(use|do|enable)insecure|(set|do|use)?no.*(check|validation|verify|verification)|disable).*$")
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
bindingset[result]
|
bindingset[result]
|
||||||
@@ -139,7 +113,12 @@ private predicate isFlag(DataFlow::Node source) {
|
|||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
/** Holds if there is flow from `node1` to `node2` either due to local flow or due to custom flow steps. */
|
/**
|
||||||
|
* Holds if there is flow from `node1` to `node2` either due to local flow or due to custom flow steps:
|
||||||
|
* 1. `Boolean.parseBoolean(taintedValue)` taints the return value of `parseBoolean`.
|
||||||
|
* 2. A call to an `EnvReadMethod` such as `System.getProperty` where a tainted value is used as an argument.
|
||||||
|
* The return value of such a method is then tainted.
|
||||||
|
*/
|
||||||
private predicate flagFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
|
private predicate flagFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
|
||||||
DataFlow::localFlowStep(node1, node2)
|
DataFlow::localFlowStep(node1, node2)
|
||||||
or
|
or
|
||||||
|
|||||||
Reference in New Issue
Block a user