Skip to content

Commit 08abdb8

Browse files
bdrodesbdrodes
committed
Crypto: Adding a "javaConstant" concept to handle config files.
1 parent e76ced1 commit 08abdb8

File tree

2 files changed

+113
-56
lines changed

2 files changed

+113
-56
lines changed

‎java/ql/lib/experimental/quantum/JCA.qll‎

Lines changed: 53 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -52,11 +52,7 @@ module JCAModel {
5252
}
5353

5454
bindingset[hash]
55-
predicate hash_names(string hash) {
56-
hash.toUpperCase()
57-
.matches(["SHA-%", "SHA3-%", "BLAKE2b%", "BLAKE2s%", "MD5", "RIPEMD160", "Whirlpool"]
58-
.toUpperCase())
59-
}
55+
predicate hash_names(string hash) { exists(hash_name_to_type_known(hash, _)) }
6056

6157
bindingset[kdf]
6258
predicate kdf_names(string kdf) {
@@ -132,41 +128,43 @@ module JCAModel {
132128
// TODO: add additional
133129
}
134130

135-
bindingset[name]
136-
Crypto::HashType hash_name_to_type_known(string name, int digestLength) {
137-
name in ["SHA-1", "SHA1"] and result instanceof Crypto::SHA1 and digestLength = 160
138-
or
139-
name in ["SHA-256", "SHA-384", "SHA-512", "SHA256", "SHA384", "SHA512"] and
140-
result instanceof Crypto::SHA2 and
141-
digestLength = name.replaceAll("-", "").splitAt("SHA", 1).toInt()
142-
or
143-
name in ["SHA3-224", "SHA3-256", "SHA3-384", "SHA3-512", "SHA3256", "SHA3384", "SHA3512"] and
144-
result instanceof Crypto::SHA3 and
145-
digestLength = name.replaceAll("-", "").splitAt("SHA3", 1).toInt()
146-
or
147-
(
148-
name.matches("BLAKE2b%") and
149-
result instanceof Crypto::BLAKE2B
131+
bindingset[nameRaw]
132+
Crypto::HashType hash_name_to_type_known(string nameRaw, int digestLength) {
133+
exists(string name | name = nameRaw.toUpperCase() |
134+
name in ["SHA-1", "SHA1"] and result instanceof Crypto::SHA1 and digestLength = 160
150135
or
151-
name = "BLAKE2s" and result instanceof Crypto::BLAKE2S
152-
) and
153-
(
154-
if exists(name.indexOf("-"))
155-
then name.splitAt("-", 1).toInt() = digestLength
156-
else digestLength = 512
136+
name in ["SHA-256", "SHA-384", "SHA-512", "SHA256", "SHA384", "SHA512"] and
137+
result instanceof Crypto::SHA2 and
138+
digestLength = name.replaceAll("-", "").splitAt("SHA", 1).toInt()
139+
or
140+
name in ["SHA3-224", "SHA3-256", "SHA3-384", "SHA3-512", "SHA3256", "SHA3384", "SHA3512"] and
141+
result instanceof Crypto::SHA3 and
142+
digestLength = name.replaceAll("-", "").splitAt("SHA3", 1).toInt()
143+
or
144+
(
145+
name.toUpperCase().matches("BLAKE2B%") and
146+
result instanceof Crypto::BLAKE2B
147+
or
148+
name.toUpperCase() = "BLAKE2S" and result instanceof Crypto::BLAKE2S
149+
) and
150+
(
151+
if exists(name.indexOf("-"))
152+
then name.splitAt("-", 1).toInt() = digestLength
153+
else digestLength = 512
154+
)
155+
or
156+
name = "MD5" and
157+
result instanceof Crypto::MD5 and
158+
digestLength = 128
159+
or
160+
name = "RIPEMD160" and
161+
result instanceof Crypto::RIPEMD160 and
162+
digestLength = 160
163+
or
164+
name = "WHIRLPOOL" and
165+
result instanceof Crypto::WHIRLPOOL and
166+
digestLength = 512 // TODO: verify
157167
)
158-
or
159-
name = "MD5" and
160-
result instanceof Crypto::MD5 and
161-
digestLength = 128
162-
or
163-
name = "RIPEMD160" and
164-
result instanceof Crypto::RIPEMD160 and
165-
digestLength = 160
166-
or
167-
name = "Whirlpool" and
168-
result instanceof Crypto::WHIRLPOOL and
169-
digestLength = 512 // TODO: verify
170168
}
171169

172170
bindingset[name]
@@ -268,9 +266,9 @@ module JCAModel {
268266
}
269267

270268
/**
271-
* A `StringLiteral` in the `"ALG/MODE/PADDING"` or `"ALG"` format
269+
* A `JavaConstant` in the `"ALG/MODE/PADDING"` or `"ALG"` format
272270
*/
273-
class CipherStringLiteral extends StringLiteral {
271+
class CipherStringLiteral extends JavaConstant {
274272
CipherStringLiteral() { cipher_names(this.getValue().splitAt("/")) }
275273

276274
string getAlgorithmName() { result = this.getValue().splitAt("/", 0) }
@@ -839,7 +837,7 @@ module JCAModel {
839837
* Flow from a known hash algorithm name to a `MessageDigest.getInstance(sink)` call.
840838
*/
841839
module KnownHashAlgorithmLiteralToMessageDigestConfig implements DataFlow::ConfigSig {
842-
predicate isSource(DataFlow::Node src) { hash_names(src.asExpr().(StringLiteral).getValue()) }
840+
predicate isSource(DataFlow::Node src) { hash_names(src.asExpr().(JavaConstant).getValue()) }
843841

844842
predicate isSink(DataFlow::Node sink) {
845843
exists(HashAlgorithmValueConsumer consumer | sink = consumer.getInputNode())
@@ -849,7 +847,7 @@ module JCAModel {
849847
module KnownHashAlgorithmLiteralToMessageDigestFlow =
850848
DataFlow::Global<KnownHashAlgorithmLiteralToMessageDigestConfig>;
851849

852-
class KnownHashAlgorithm extends Crypto::HashAlgorithmInstance instanceof StringLiteral {
850+
class KnownHashAlgorithm extends Crypto::HashAlgorithmInstance instanceof JavaConstant {
853851
HashAlgorithmValueConsumer consumer;
854852

855853
KnownHashAlgorithm() {
@@ -1195,7 +1193,7 @@ module JCAModel {
11951193
}
11961194

11971195
module KDFAlgorithmStringToGetInstanceConfig implements DataFlow::ConfigSig {
1198-
predicate isSource(DataFlow::Node src) { kdf_names(src.asExpr().(StringLiteral).getValue()) }
1196+
predicate isSource(DataFlow::Node src) { kdf_names(src.asExpr().(JavaConstant).getValue()) }
11991197

12001198
predicate isSink(DataFlow::Node sink) {
12011199
exists(SecretKeyFactoryGetInstanceCall call | sink.asExpr() = call.getAlgorithmArg())
@@ -1236,7 +1234,7 @@ module JCAModel {
12361234
predicate isIntermediate() { none() }
12371235
}
12381236

1239-
class KdfAlgorithmStringLiteral extends Crypto::KeyDerivationAlgorithmInstance instanceof StringLiteral
1237+
class KdfAlgorithmStringLiteral extends Crypto::KeyDerivationAlgorithmInstance instanceof JavaConstant
12401238
{
12411239
SecretKeyFactoryKDFAlgorithmValueConsumer consumer;
12421240

@@ -1257,7 +1255,7 @@ module JCAModel {
12571255
class Pbkdf2WithHmac_KeyOperationAlgorithmStringLiteral extends Crypto::KeyOperationAlgorithmInstance instanceof KdfAlgorithmStringLiteral
12581256
{
12591257
Pbkdf2WithHmac_KeyOperationAlgorithmStringLiteral() {
1260-
this.(StringLiteral).getValue().toUpperCase().matches("PBKDF2WithHmac%".toUpperCase())
1258+
this.(JavaConstant).getValue().toUpperCase().matches("PBKDF2WithHmac%".toUpperCase())
12611259
}
12621260

12631261
override Crypto::KeyOpAlg::AlgorithmType getAlgorithmType() {
@@ -1278,18 +1276,18 @@ module JCAModel {
12781276

12791277
override Crypto::PaddingAlgorithmInstance getPaddingAlgorithm() { none() }
12801278

1281-
override string getRawAlgorithmName() { result = this.(StringLiteral).getValue() }
1279+
override string getRawAlgorithmName() { result = this.(JavaConstant).getValue() }
12821280
}
12831281

12841282
class Pbkdf2WithHmac_HashAlgorithmStringLiteral extends Crypto::HashAlgorithmInstance instanceof Pbkdf2WithHmac_KeyOperationAlgorithmStringLiteral
12851283
{
12861284
string hashName;
12871285

12881286
Pbkdf2WithHmac_HashAlgorithmStringLiteral() {
1289-
hashName = this.(StringLiteral).getValue().splitAt("WithHmac", 1)
1287+
hashName = this.(JavaConstant).getValue().splitAt("WithHmac", 1)
12901288
}
12911289

1292-
override string getRawHashAlgorithmName() { result = this.(StringLiteral).getValue() }
1290+
override string getRawHashAlgorithmName() { result = this.(JavaConstant).getValue() }
12931291

12941292
override Crypto::THashType getHashType() { result = hash_name_to_type_known(hashName, _) }
12951293

@@ -1403,7 +1401,7 @@ module JCAModel {
14031401
GetInstanceInitUseFlowAnalysis<KeyAgreementGetInstanceCall, KeyAgreementInitCall,
14041402
KeyAgreementCall>;
14051403

1406-
class KeyAgreementStringLiteral extends StringLiteral {
1404+
class KeyAgreementStringLiteral extends JavaConstant {
14071405
KeyAgreementStringLiteral() { key_agreement_names(this.getValue()) }
14081406
}
14091407

@@ -1521,7 +1519,7 @@ module JCAModel {
15211519
*/
15221520

15231521
module MacKnownAlgorithmToConsumerConfig implements DataFlow::ConfigSig {
1524-
predicate isSource(DataFlow::Node src) { mac_names(src.asExpr().(StringLiteral).getValue()) }
1522+
predicate isSource(DataFlow::Node src) { mac_names(src.asExpr().(JavaConstant).getValue()) }
15251523

15261524
predicate isSink(DataFlow::Node sink) {
15271525
exists(MacGetInstanceCall call | sink.asExpr() = call.getAlgorithmArg())
@@ -1555,7 +1553,7 @@ module JCAModel {
15551553

15561554
module MacInitCallToMacOperationFlow = DataFlow::Global<MacInitCallToMacOperationFlowConfig>;
15571555

1558-
class KnownMacAlgorithm extends Crypto::KeyOperationAlgorithmInstance instanceof StringLiteral {
1556+
class KnownMacAlgorithm extends Crypto::KeyOperationAlgorithmInstance instanceof JavaConstant {
15591557
MacGetInstanceAlgorithmValueConsumer consumer;
15601558

15611559
KnownMacAlgorithm() {
@@ -1711,7 +1709,7 @@ module JCAModel {
17111709
}
17121710
}
17131711

1714-
class SignatureStringLiteral extends StringLiteral {
1712+
class SignatureStringLiteral extends JavaConstant {
17151713
SignatureStringLiteral() { signature_names(this.getValue()) }
17161714
}
17171715

@@ -1754,10 +1752,10 @@ module JCAModel {
17541752
int digestLength;
17551753

17561754
SignatureHashAlgorithmInstance() {
1757-
hashType = signature_name_to_hash_type_known(this.(StringLiteral).getValue(), digestLength)
1755+
hashType = signature_name_to_hash_type_known(this.(JavaConstant).getValue(), digestLength)
17581756
}
17591757

1760-
override string getRawHashAlgorithmName() { result = this.(StringLiteral).getValue() }
1758+
override string getRawHashAlgorithmName() { result = this.(JavaConstant).getValue() }
17611759

17621760
override Crypto::THashType getHashType() { result = hashType }
17631761

@@ -1880,7 +1878,7 @@ module JCAModel {
18801878

18811879
module EllipticCurveStringToConsumerFlow = DataFlow::Global<EllipticCurveStringToConsumerConfig>;
18821880

1883-
class EllipticCurveStringLiteral extends StringLiteral {
1881+
class EllipticCurveStringLiteral extends JavaConstant {
18841882
EllipticCurveStringLiteral() { elliptic_curve_names(this.getValue()) }
18851883
}
18861884

‎java/ql/lib/experimental/quantum/Language.qll‎

Lines changed: 60 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,66 @@ private class GenericRemoteDataSource extends Crypto::GenericRemoteDataSource {
9393
override string getAdditionalDescription() { result = this.toString() }
9494
}
9595

96-
private class ConstantDataSourceLiteral extends Crypto::GenericConstantSourceInstance instanceof Literal
96+
// /**
97+
// * A property access value (constant from a file)
98+
// */
99+
// class PropertyConstant extends Crypto::GenericConstantSourceInstance instanceof Literal{
100+
// PropertyConstant() {
101+
// value = this.getPropertyValue() and
102+
// // Since properties pairs are not included in the java/weak-cryptographic-algorithm,
103+
// // the check for values from properties files can be less strict than `InsecureAlgoLiteral`.
104+
// not value.regexpMatch(getSecureAlgorithmRegex())
105+
// }
106+
// override string getStringValue() { result = value }
107+
// }
108+
import semmle.code.java.dataflow.RangeUtils
109+
// TODO: import all frameworks?
110+
import semmle.code.java.frameworks.Properties
111+
private import semmle.code.configfiles.ConfigFiles
112+
113+
/**
114+
* A class to represent constants in Java code, either literals or
115+
* values retrieved from properties files.
116+
* Java CodeQL does not consider the values of known properties to be literals,
117+
* hence we need to model both literals and property calls.
118+
*/
119+
class JavaConstant extends Expr {
120+
string value;
121+
122+
JavaConstant() {
123+
// If arg 0 in a getProperty call, consider it a literal only if
124+
// we haven't resolved it to a known property value, otherwise
125+
// use the resolved config value.
126+
// If getProperty is used, always assume the default value is potentially used.
127+
// CAVEAT/ASSUMPTION: this assumes the literal is immediately known at arg0
128+
// of a getProperty call.
129+
// also if the properties file is reloaded in a way where the reloaded file
130+
// wouldn't have the property but the original does, we would erroneously
131+
// consider the literal to be mapped to that property value.
132+
exists(ConfigPair p, PropertiesGetPropertyMethodCall c |
133+
c.getArgument(0).(Literal).getValue() = p.getNameElement().getName() and
134+
value = p.getValueElement().getValue() and
135+
this = c
136+
)
137+
or
138+
// in this case, the property value is not known, use the literal property name as the value
139+
exists(PropertiesGetPropertyMethodCall c |
140+
value = c.getArgument(0).(Literal).getValue() and
141+
not exists(ConfigPair p |
142+
c.getArgument(0).(Literal).getValue() = p.getNameElement().getName()
143+
) and
144+
this = c
145+
)
146+
or
147+
// in this case, there is not propery getter, we just have a literal
148+
not exists(PropertiesGetPropertyMethodCall c | c.getArgument(0) = this) and
149+
value = this.(Literal).getValue()
150+
}
151+
152+
string getValue() { result = value }
153+
}
154+
155+
private class ConstantDataSourceLiteral extends Crypto::GenericConstantSourceInstance instanceof JavaConstant
97156
{
98157
ConstantDataSourceLiteral() {
99158
// TODO: this is an API specific workaround for JCA, as 'EC' is a constant that may be used

0 commit comments

Comments
 (0)