Java: Generelize MaybeBrokenCryptoAlgorithmQuery.qll

This commit is contained in:
Tony Torralba
2023-12-22 10:15:40 +01:00
parent 8051cfcef5
commit 8ad787f3b8
2 changed files with 29 additions and 29 deletions

View File

@@ -10,6 +10,12 @@ private import semmle.code.java.dataflow.RangeUtils
private import semmle.code.java.dispatch.VirtualDispatch
private import semmle.code.java.frameworks.Properties
/** A reference to an insecure cryptographic algorithm. */
abstract class InsecureAlgorithm extends Expr {
/** Gets the string representation of this insecure cryptographic algorithm. */
abstract string getStringValue();
}
private class ShortStringLiteral extends StringLiteral {
ShortStringLiteral() { this.getValue().length() < 100 }
}
@@ -17,16 +23,34 @@ private class ShortStringLiteral extends StringLiteral {
/**
* A string literal that may refer to an insecure cryptographic algorithm.
*/
class InsecureAlgoLiteral extends ShortStringLiteral {
class InsecureAlgoLiteral extends InsecureAlgorithm, ShortStringLiteral {
InsecureAlgoLiteral() {
// Algorithm identifiers should be at least two characters.
this.getValue().length() > 1 and
exists(string s | s = this.getValue() |
// Algorithm identifiers should be at least two characters.
s.length() > 1 and
not s.regexpMatch(getSecureAlgorithmRegex()) and
// Exclude results covered by another query.
not s.regexpMatch(getInsecureAlgorithmRegex())
)
}
override string getStringValue() { result = this.getValue() }
}
/**
* A property access that may refer to an insecure cryptographic algorithm.
*/
class InsecureAlgoProperty extends InsecureAlgorithm, PropertiesGetPropertyMethodCall {
string value;
InsecureAlgoProperty() {
value = this.getPropertyValue() and
// Since properties pairs are not included in the java/weak-cryptographic-algorithm,
// the check for values from properties files can be less strict than `InsecureAlgoLiteral`.
not value.regexpMatch(getSecureAlgorithmRegex())
}
override string getStringValue() { result = value }
}
private predicate objectToString(MethodCall ma) {
@@ -41,17 +65,7 @@ private predicate objectToString(MethodCall ma) {
* A taint-tracking configuration to reason about the use of potentially insecure cryptographic algorithms.
*/
module InsecureCryptoConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node n) {
n.asExpr() instanceof InsecureAlgoLiteral
or
exists(PropertiesGetPropertyMethodCall mc, string value |
n.asExpr() = mc and value = mc.getPropertyValue()
|
// Since properties pairs are not included in the java/weak-crypto-algorithm,
// The check for values from properties files can be less strict than `InsecureAlgoLiteral`.
not value.regexpMatch(getSecureAlgorithmRegex())
)
}
predicate isSource(DataFlow::Node n) { n.asExpr() instanceof InsecureAlgorithm }
predicate isSink(DataFlow::Node n) { exists(CryptoAlgoSpec c | n.asExpr() = c.getAlgoSpec()) }

View File

@@ -18,24 +18,10 @@ import semmle.code.java.frameworks.Properties
import semmle.code.java.security.MaybeBrokenCryptoAlgorithmQuery
import InsecureCryptoFlow::PathGraph
/**
* Get the string value represented by the given expression.
*
* If the value is a string literal, get the literal value.
* If the value is a call to `java.util.Properties::getProperty`, get the potential values of the property.
*/
string getStringValue(DataFlow::Node algo) {
result = algo.asExpr().(StringLiteral).getValue()
or
exists(string value | value = algo.asExpr().(PropertiesGetPropertyMethodCall).getPropertyValue() |
result = value and not value.regexpMatch(getSecureAlgorithmRegex())
)
}
from InsecureCryptoFlow::PathNode source, InsecureCryptoFlow::PathNode sink, CryptoAlgoSpec c
where
sink.getNode().asExpr() = c.getAlgoSpec() and
InsecureCryptoFlow::flowPath(source, sink)
select c, source, sink,
"Cryptographic algorithm $@ may not be secure, consider using a different algorithm.", source,
getStringValue(source.getNode())
source.getNode().asExpr().(InsecureAlgorithm).getStringValue()