Skip to content

Commit 83c6ece

Browse files
committed
Move weak hashing into MaybeBrokenCryptoAlgorithm
1 parent fbc2a33 commit 83c6ece

File tree

7 files changed

+34
-123
lines changed

7 files changed

+34
-123
lines changed

java/ql/lib/semmle/code/java/security/MaybeBrokenCryptoAlgorithmQuery.qll

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,12 @@
33
*/
44

55
import java
6+
private import semmle.code.configfiles.ConfigFiles
67
private import semmle.code.java.security.Encryption
78
private import semmle.code.java.dataflow.TaintTracking
9+
private import semmle.code.java.dataflow.RangeUtils
810
private import semmle.code.java.dispatch.VirtualDispatch
11+
private import semmle.code.java.frameworks.Properties
912

1013
private class ShortStringLiteral extends StringLiteral {
1114
ShortStringLiteral() { this.getValue().length() < 100 }
@@ -34,11 +37,38 @@ private predicate objectToString(MethodCall ma) {
3437
)
3538
}
3639

40+
private class GetPropertyMethodCall extends MethodCall {
41+
GetPropertyMethodCall() { this.getMethod() instanceof PropertiesGetPropertyMethod }
42+
43+
private ConfigPair getPair() {
44+
this.getArgument(0).(ConstantStringExpr).getStringValue() = result.getNameElement().getName()
45+
}
46+
47+
string getPropertyValue() {
48+
result = this.getPair().getValueElement().getValue() or
49+
result = this.getArgument(1).(ConstantStringExpr).getStringValue()
50+
}
51+
}
52+
53+
string insecureAlgorithmName(DataFlow::Node algo) {
54+
result = algo.asExpr().(StringLiteral).getValue()
55+
or
56+
result = algo.asExpr().(GetPropertyMethodCall).getPropertyValue()
57+
}
58+
3759
/**
3860
* A taint-tracking configuration to reason about the use of potentially insecure cryptographic algorithms.
3961
*/
4062
module InsecureCryptoConfig implements DataFlow::ConfigSig {
41-
predicate isSource(DataFlow::Node n) { n.asExpr() instanceof InsecureAlgoLiteral }
63+
predicate isSource(DataFlow::Node n) {
64+
n.asExpr() instanceof InsecureAlgoLiteral
65+
or
66+
exists(GetPropertyMethodCall mc | n.asExpr() = mc |
67+
// Since properties pairs are not included in the java/weak-crypto-algorithm,
68+
// The check for values from properties files can be less strict than `InsecureAlgoLiteral`.
69+
not mc.getPropertyValue().regexpMatch(getSecureAlgorithmRegex())
70+
)
71+
}
4272

4373
predicate isSink(DataFlow::Node n) { exists(CryptoAlgoSpec c | n.asExpr() = c.getAlgoSpec()) }
4474

java/ql/lib/semmle/code/java/security/WeakHashingAlgorithmPropertyQuery.qll

Lines changed: 0 additions & 54 deletions
This file was deleted.

java/ql/src/Security/CWE/CWE-327/MaybeBrokenCryptoAlgorithm.ql

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,10 @@ import semmle.code.java.security.Encryption
1616
import semmle.code.java.security.MaybeBrokenCryptoAlgorithmQuery
1717
import InsecureCryptoFlow::PathGraph
1818

19-
from
20-
InsecureCryptoFlow::PathNode source, InsecureCryptoFlow::PathNode sink, CryptoAlgoSpec c,
21-
InsecureAlgoLiteral s
19+
from InsecureCryptoFlow::PathNode source, InsecureCryptoFlow::PathNode sink, CryptoAlgoSpec c
2220
where
2321
sink.getNode().asExpr() = c.getAlgoSpec() and
24-
source.getNode().asExpr() = s and
2522
InsecureCryptoFlow::flowPath(source, sink)
2623
select c, source, sink,
27-
"Cryptographic algorithm $@ may not be secure, consider using a different algorithm.", s,
28-
s.getValue()
24+
"Cryptographic algorithm $@ may not be secure, consider using a different algorithm.", source,
25+
insecureAlgorithmName(source.getNode())

java/ql/src/Security/CWE/CWE-328/WeakHashingProperty.java

Lines changed: 0 additions & 9 deletions
This file was deleted.

java/ql/src/Security/CWE/CWE-328/WeakHashingProperty.qhelp

Lines changed: 0 additions & 32 deletions
This file was deleted.

java/ql/src/Security/CWE/CWE-328/WeakHashingProperty.ql

Lines changed: 0 additions & 20 deletions
This file was deleted.

java/ql/src/Security/CWE/CWE-328/settings.properties

Lines changed: 0 additions & 1 deletion
This file was deleted.

0 commit comments

Comments
 (0)