Skip to content

Commit f42bd28

Browse files
author
Max Schaefer
committed
Port changes to Ruby.
1 parent 741735c commit f42bd28

File tree

5 files changed

+55
-37
lines changed

5 files changed

+55
-37
lines changed

‎ruby/ql/lib/codeql/ruby/frameworks/core/Digest.qll

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,15 +18,21 @@ private API::Node digest(Cryptography::HashingAlgorithm algo) {
1818
private class DigestCall extends Cryptography::CryptographicOperation::Range instanceof DataFlow::CallNode
1919
{
2020
Cryptography::HashingAlgorithm algo;
21+
API::Node digestNode;
2122

2223
DigestCall() {
23-
this = digest(algo).getAMethodCall(["hexdigest", "base64digest", "bubblebabble"])
24-
or
25-
this = digest(algo).getAMethodCall("file") // it's directly hashing the contents of a file, but that's close enough for us.
26-
or
27-
this = digest(algo).getInstance().getAMethodCall(["digest", "update", "<<"])
24+
digestNode = digest(algo) and
25+
(
26+
this = digestNode.getAMethodCall(["hexdigest", "base64digest", "bubblebabble"])
27+
or
28+
this = digestNode.getAMethodCall("file") // it's directly hashing the contents of a file, but that's close enough for us.
29+
or
30+
this = digestNode.getInstance().getAMethodCall(["digest", "update", "<<"])
31+
)
2832
}
2933

34+
override DataFlow::Node getInitialization() { result = digestNode.asSource() }
35+
3036
override Cryptography::HashingAlgorithm getAlgorithm() { result = algo }
3137

3238
override DataFlow::Node getAnInput() { result = super.getArgument(0) }

‎ruby/ql/lib/codeql/ruby/internal/ConceptsShared.qll

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,9 @@ module Cryptography {
4040
/** Gets the algorithm used, if it matches a known `CryptographicAlgorithm`. */
4141
CryptographicAlgorithm getAlgorithm() { result = super.getAlgorithm() }
4242

43+
/** Gets the data-flow node where the cryptographic algorithm used in this operation is configured. */
44+
DataFlow::Node getInitialization() { result = super.getInitialization() }
45+
4346
/** Gets an input the algorithm is used on, for example the plain text input to be encrypted. */
4447
DataFlow::Node getAnInput() { result = super.getAnInput() }
4548

@@ -65,6 +68,9 @@ module Cryptography {
6568
* extend `CryptographicOperation` instead.
6669
*/
6770
abstract class Range extends DataFlow::Node {
71+
/** Gets the data-flow node where the cryptographic algorithm used in this operation is configured. */
72+
abstract DataFlow::Node getInitialization();
73+
6874
/** Gets the algorithm used, if it matches a known `CryptographicAlgorithm`. */
6975
abstract CryptographicAlgorithm getAlgorithm();
7076

‎ruby/ql/lib/codeql/ruby/security/OpenSSL.qll

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -569,6 +569,8 @@ private class CipherOperation extends Cryptography::CryptographicOperation::Rang
569569
this.getMethodName() = "update"
570570
}
571571

572+
override DataFlow::Node getInitialization() { result = cipherNode }
573+
572574
override Cryptography::EncryptionAlgorithm getAlgorithm() {
573575
result = cipherNode.getCipher().getAlgorithm()
574576
}
@@ -591,21 +593,21 @@ private module Digest {
591593
private class DigestCall extends Cryptography::CryptographicOperation::Range instanceof DataFlow::CallNode
592594
{
593595
Cryptography::HashingAlgorithm algo;
596+
API::MethodAccessNode call;
594597

595598
DigestCall() {
596-
exists(API::MethodAccessNode call |
597-
call = API::getTopLevelMember("OpenSSL").getMember("Digest").getMethod("new")
598-
|
599-
this = call.getReturn().getAMethodCall(["digest", "update", "<<"]) and
600-
algo.matchesName(call.asCall()
601-
.getArgument(0)
602-
.asExpr()
603-
.getExpr()
604-
.getConstantValue()
605-
.getString())
606-
)
599+
call = API::getTopLevelMember("OpenSSL").getMember("Digest").getMethod("new") and
600+
this = call.getReturn().getAMethodCall(["digest", "update", "<<"]) and
601+
algo.matchesName(call.asCall()
602+
.getArgument(0)
603+
.asExpr()
604+
.getExpr()
605+
.getConstantValue()
606+
.getString())
607607
}
608608

609+
override DataFlow::Node getInitialization() { result = call.asCall() }
610+
609611
override Cryptography::HashingAlgorithm getAlgorithm() { result = algo }
610612

611613
override DataFlow::Node getAnInput() { result = super.getArgument(0) }
@@ -617,12 +619,16 @@ private module Digest {
617619
private class DigestCallDirect extends Cryptography::CryptographicOperation::Range instanceof DataFlow::CallNode
618620
{
619621
Cryptography::HashingAlgorithm algo;
622+
API::Node digestNode;
620623

621624
DigestCallDirect() {
622-
this = API::getTopLevelMember("OpenSSL").getMember("Digest").getMethod("digest").asCall() and
625+
digestNode = API::getTopLevelMember("OpenSSL").getMember("Digest") and
626+
this = digestNode.getMethod("digest").asCall() and
623627
algo.matchesName(this.getArgument(0).asExpr().getExpr().getConstantValue().getString())
624628
}
625629

630+
override DataFlow::Node getInitialization() { result = digestNode.asSource() }
631+
626632
override Cryptography::HashingAlgorithm getAlgorithm() { result = algo }
627633

628634
override DataFlow::Node getAnInput() { result = super.getArgument(1) }

‎ruby/ql/src/queries/security/cwe-327/BrokenCryptoAlgorithm.ql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,4 +23,4 @@ where
2323
)
2424
or
2525
operation.getBlockMode().isWeak() and msgPrefix = "The block mode " + operation.getBlockMode()
26-
select operation, msgPrefix + " is broken or weak, and should not be used."
26+
select operation, msgPrefix + " (configured $@) is broken or weak, and should not be used.", operation.getInitialization(), "here"

‎ruby/ql/test/query-tests/security/cwe-327/BrokenCryptoAlgorithm.expected

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,19 @@
1-
| broken_crypto.rb:4:8:4:34 | call to new | The cryptographic algorithm DES is broken or weak, and should not be used. |
2-
| broken_crypto.rb:8:1:8:18 | call to update | The cryptographic algorithm DES is broken or weak, and should not be used. |
3-
| broken_crypto.rb:12:8:12:43 | call to new | The block mode ECB is broken or weak, and should not be used. |
4-
| broken_crypto.rb:16:1:16:18 | call to update | The block mode ECB is broken or weak, and should not be used. |
5-
| broken_crypto.rb:28:1:28:35 | call to new | The block mode ECB is broken or weak, and should not be used. |
6-
| broken_crypto.rb:37:1:37:33 | call to new | The block mode ECB is broken or weak, and should not be used. |
7-
| broken_crypto.rb:42:1:42:33 | call to new | The block mode ECB is broken or weak, and should not be used. |
8-
| broken_crypto.rb:47:1:47:33 | call to new | The block mode ECB is broken or weak, and should not be used. |
9-
| broken_crypto.rb:52:1:52:29 | call to new | The block mode ECB is broken or weak, and should not be used. |
10-
| broken_crypto.rb:57:1:57:32 | call to new | The block mode ECB is broken or weak, and should not be used. |
11-
| broken_crypto.rb:60:1:60:24 | call to new | The cryptographic algorithm DES is broken or weak, and should not be used. |
12-
| broken_crypto.rb:62:1:62:30 | call to new | The cryptographic algorithm DES is broken or weak, and should not be used. |
13-
| broken_crypto.rb:67:1:67:31 | call to new | The block mode ECB is broken or weak, and should not be used. |
14-
| broken_crypto.rb:70:1:70:24 | call to new | The cryptographic algorithm RC2 is broken or weak, and should not be used. |
15-
| broken_crypto.rb:72:1:72:30 | call to new | The block mode ECB is broken or weak, and should not be used. |
16-
| broken_crypto.rb:72:1:72:30 | call to new | The cryptographic algorithm RC2 is broken or weak, and should not be used. |
17-
| broken_crypto.rb:75:1:75:24 | call to new | The cryptographic algorithm RC4 is broken or weak, and should not be used. |
18-
| broken_crypto.rb:77:1:77:29 | call to new | The cryptographic algorithm RC4 is broken or weak, and should not be used. |
19-
| broken_crypto.rb:79:1:79:35 | call to new | The cryptographic algorithm RC4 is broken or weak, and should not be used. |
1+
| broken_crypto.rb:4:8:4:34 | call to new | The cryptographic algorithm DES (configured $@) is broken or weak, and should not be used. | broken_crypto.rb:4:8:4:34 | call to new | here |
2+
| broken_crypto.rb:8:1:8:18 | call to update | The cryptographic algorithm DES (configured $@) is broken or weak, and should not be used. | broken_crypto.rb:8:1:8:4 | weak | here |
3+
| broken_crypto.rb:12:8:12:43 | call to new | The block mode ECB (configured $@) is broken or weak, and should not be used. | broken_crypto.rb:12:8:12:43 | call to new | here |
4+
| broken_crypto.rb:16:1:16:18 | call to update | The block mode ECB (configured $@) is broken or weak, and should not be used. | broken_crypto.rb:16:1:16:4 | weak | here |
5+
| broken_crypto.rb:28:1:28:35 | call to new | The block mode ECB (configured $@) is broken or weak, and should not be used. | broken_crypto.rb:28:1:28:35 | call to new | here |
6+
| broken_crypto.rb:37:1:37:33 | call to new | The block mode ECB (configured $@) is broken or weak, and should not be used. | broken_crypto.rb:37:1:37:33 | call to new | here |
7+
| broken_crypto.rb:42:1:42:33 | call to new | The block mode ECB (configured $@) is broken or weak, and should not be used. | broken_crypto.rb:42:1:42:33 | call to new | here |
8+
| broken_crypto.rb:47:1:47:33 | call to new | The block mode ECB (configured $@) is broken or weak, and should not be used. | broken_crypto.rb:47:1:47:33 | call to new | here |
9+
| broken_crypto.rb:52:1:52:29 | call to new | The block mode ECB (configured $@) is broken or weak, and should not be used. | broken_crypto.rb:52:1:52:29 | call to new | here |
10+
| broken_crypto.rb:57:1:57:32 | call to new | The block mode ECB (configured $@) is broken or weak, and should not be used. | broken_crypto.rb:57:1:57:32 | call to new | here |
11+
| broken_crypto.rb:60:1:60:24 | call to new | The cryptographic algorithm DES (configured $@) is broken or weak, and should not be used. | broken_crypto.rb:60:1:60:24 | call to new | here |
12+
| broken_crypto.rb:62:1:62:30 | call to new | The cryptographic algorithm DES (configured $@) is broken or weak, and should not be used. | broken_crypto.rb:62:1:62:30 | call to new | here |
13+
| broken_crypto.rb:67:1:67:31 | call to new | The block mode ECB (configured $@) is broken or weak, and should not be used. | broken_crypto.rb:67:1:67:31 | call to new | here |
14+
| broken_crypto.rb:70:1:70:24 | call to new | The cryptographic algorithm RC2 (configured $@) is broken or weak, and should not be used. | broken_crypto.rb:70:1:70:24 | call to new | here |
15+
| broken_crypto.rb:72:1:72:30 | call to new | The block mode ECB (configured $@) is broken or weak, and should not be used. | broken_crypto.rb:72:1:72:30 | call to new | here |
16+
| broken_crypto.rb:72:1:72:30 | call to new | The cryptographic algorithm RC2 (configured $@) is broken or weak, and should not be used. | broken_crypto.rb:72:1:72:30 | call to new | here |
17+
| broken_crypto.rb:75:1:75:24 | call to new | The cryptographic algorithm RC4 (configured $@) is broken or weak, and should not be used. | broken_crypto.rb:75:1:75:24 | call to new | here |
18+
| broken_crypto.rb:77:1:77:29 | call to new | The cryptographic algorithm RC4 (configured $@) is broken or weak, and should not be used. | broken_crypto.rb:77:1:77:29 | call to new | here |
19+
| broken_crypto.rb:79:1:79:35 | call to new | The cryptographic algorithm RC4 (configured $@) is broken or weak, and should not be used. | broken_crypto.rb:79:1:79:35 | call to new | here |

0 commit comments

Comments
 (0)