Skip to content

Commit b968b59

Browse files
committed
CryptoAlgorithms: make CryptographicAlgorithm#matchesName hold only if that algorithm is the most specific match
1 parent e17b3d9 commit b968b59

File tree

3 files changed

+69
-21
lines changed

3 files changed

+69
-21
lines changed

javascript/ql/lib/semmle/javascript/security/CryptoAlgorithms.qll

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,26 @@ private newtype TCryptographicAlgorithm =
2626
isWeakPasswordHashingAlgorithm(name) and isWeak = true
2727
}
2828

29+
/**
30+
* Gets the most specific `CryptographicAlgorithm` that matches the given `name`.
31+
* A matching algorithm is one where the name of the algorithm matches the start of name, with allowances made for different name formats.
32+
* In the case that multiple `CryptographicAlgorithm`s match the given `name`, the algorithm(s) with the longest name will be selected. This is intended to select more specific versions of algorithms when multiple versions could match - for example "SHA3_224" matches against both "SHA3" and "SHA3224", but the latter is a more precise match.
33+
*/
34+
bindingset[name]
35+
private CryptographicAlgorithm getBestAlgorithmForName(string name) {
36+
result =
37+
max(CryptographicAlgorithm algorithm |
38+
algorithm.getName() =
39+
[
40+
name.toUpperCase(), // the full name
41+
name.toUpperCase().regexpCapture("^([\\w]+)(?:-.*)?$", 1), // the name prior to any dashes or spaces
42+
name.toUpperCase().regexpCapture("^([A-Z0-9]+)(?:(-|_).*)?$", 1) // the name prior to any dashes, spaces, or underscores
43+
].regexpReplaceAll("[-_ ]", "") // strip dashes, underscores, and spaces
44+
|
45+
algorithm order by algorithm.getName().length()
46+
)
47+
}
48+
2949
/**
3050
* A cryptographic algorithm.
3151
*/
@@ -39,15 +59,11 @@ abstract class CryptographicAlgorithm extends TCryptographicAlgorithm {
3959
abstract string getName();
4060

4161
/**
42-
* Holds if the name of this algorithm matches `name` modulo case,
43-
* white space, dashes, underscores, and anything after a dash or underscore in the name
44-
* (to ignore modes of operation, such as CBC or ECB).
62+
* Holds if the name of this algorithm is the most specific match for `name`.
63+
* This predicate matches quite liberally to account for different ways of formatting algorithm names, e.g. using dashes, underscores, or spaces as separators, including or not including block modes of operation, etc.
4564
*/
4665
bindingset[name]
47-
predicate matchesName(string name) {
48-
[name.toUpperCase(), name.toUpperCase().regexpCapture("^([A-Z0-9]+)(?:(-|_).*)?$", 1)]
49-
.regexpReplaceAll("[-_ ]", "") = getName()
50-
}
66+
predicate matchesName(string name) { this = getBestAlgorithmForName(name) }
5167

5268
/**
5369
* Holds if this algorithm is weak.

python/ql/lib/semmle/python/concepts/CryptoAlgorithms.qll

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,26 @@ private newtype TCryptographicAlgorithm =
2626
isWeakPasswordHashingAlgorithm(name) and isWeak = true
2727
}
2828

29+
/**
30+
* Gets the most specific `CryptographicAlgorithm` that matches the given `name`.
31+
* A matching algorithm is one where the name of the algorithm matches the start of name, with allowances made for different name formats.
32+
* In the case that multiple `CryptographicAlgorithm`s match the given `name`, the algorithm(s) with the longest name will be selected. This is intended to select more specific versions of algorithms when multiple versions could match - for example "SHA3_224" matches against both "SHA3" and "SHA3224", but the latter is a more precise match.
33+
*/
34+
bindingset[name]
35+
private CryptographicAlgorithm getBestAlgorithmForName(string name) {
36+
result =
37+
max(CryptographicAlgorithm algorithm |
38+
algorithm.getName() =
39+
[
40+
name.toUpperCase(), // the full name
41+
name.toUpperCase().regexpCapture("^([\\w]+)(?:-.*)?$", 1), // the name prior to any dashes or spaces
42+
name.toUpperCase().regexpCapture("^([A-Z0-9]+)(?:(-|_).*)?$", 1) // the name prior to any dashes, spaces, or underscores
43+
].regexpReplaceAll("[-_ ]", "") // strip dashes, underscores, and spaces
44+
|
45+
algorithm order by algorithm.getName().length()
46+
)
47+
}
48+
2949
/**
3050
* A cryptographic algorithm.
3151
*/
@@ -39,15 +59,11 @@ abstract class CryptographicAlgorithm extends TCryptographicAlgorithm {
3959
abstract string getName();
4060

4161
/**
42-
* Holds if the name of this algorithm matches `name` modulo case,
43-
* white space, dashes, underscores, and anything after a dash or underscore in the name
44-
* (to ignore modes of operation, such as CBC or ECB).
62+
* Holds if the name of this algorithm is the most specific match for `name`.
63+
* This predicate matches quite liberally to account for different ways of formatting algorithm names, e.g. using dashes, underscores, or spaces as separators, including or not including block modes of operation, etc.
4564
*/
4665
bindingset[name]
47-
predicate matchesName(string name) {
48-
[name.toUpperCase(), name.toUpperCase().regexpCapture("^([A-Z0-9]+)(?:(-|_).*)?$", 1)]
49-
.regexpReplaceAll("[-_ ]", "") = getName()
50-
}
66+
predicate matchesName(string name) { this = getBestAlgorithmForName(name) }
5167

5268
/**
5369
* Holds if this algorithm is weak.

ruby/ql/lib/codeql/ruby/security/CryptoAlgorithms.qll

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,26 @@ private newtype TCryptographicAlgorithm =
2626
isWeakPasswordHashingAlgorithm(name) and isWeak = true
2727
}
2828

29+
/**
30+
* Gets the most specific `CryptographicAlgorithm` that matches the given `name`.
31+
* A matching algorithm is one where the name of the algorithm matches the start of name, with allowances made for different name formats.
32+
* In the case that multiple `CryptographicAlgorithm`s match the given `name`, the algorithm(s) with the longest name will be selected. This is intended to select more specific versions of algorithms when multiple versions could match - for example "SHA3_224" matches against both "SHA3" and "SHA3224", but the latter is a more precise match.
33+
*/
34+
bindingset[name]
35+
private CryptographicAlgorithm getBestAlgorithmForName(string name) {
36+
result =
37+
max(CryptographicAlgorithm algorithm |
38+
algorithm.getName() =
39+
[
40+
name.toUpperCase(), // the full name
41+
name.toUpperCase().regexpCapture("^([\\w]+)(?:-.*)?$", 1), // the name prior to any dashes or spaces
42+
name.toUpperCase().regexpCapture("^([A-Z0-9]+)(?:(-|_).*)?$", 1) // the name prior to any dashes, spaces, or underscores
43+
].regexpReplaceAll("[-_ ]", "") // strip dashes, underscores, and spaces
44+
|
45+
algorithm order by algorithm.getName().length()
46+
)
47+
}
48+
2949
/**
3050
* A cryptographic algorithm.
3151
*/
@@ -39,15 +59,11 @@ abstract class CryptographicAlgorithm extends TCryptographicAlgorithm {
3959
abstract string getName();
4060

4161
/**
42-
* Holds if the name of this algorithm matches `name` modulo case,
43-
* white space, dashes, underscores, and anything after a dash or underscore in the name
44-
* (to ignore modes of operation, such as CBC or ECB).
62+
* Holds if the name of this algorithm is the most specific match for `name`.
63+
* This predicate matches quite liberally to account for different ways of formatting algorithm names, e.g. using dashes, underscores, or spaces as separators, including or not including block modes of operation, etc.
4564
*/
4665
bindingset[name]
47-
predicate matchesName(string name) {
48-
[name.toUpperCase(), name.toUpperCase().regexpCapture("^([A-Z0-9]+)(?:(-|_).*)?$", 1)]
49-
.regexpReplaceAll("[-_ ]", "") = getName()
50-
}
66+
predicate matchesName(string name) { this = getBestAlgorithmForName(name) }
5167

5268
/**
5369
* Holds if this algorithm is weak.

0 commit comments

Comments
 (0)