Skip to content

Commit 7c76645

Browse files
committed
add model for the core OpenSSL::Digest module
1 parent e247694 commit 7c76645

File tree

4 files changed

+58
-2
lines changed

4 files changed

+58
-2
lines changed

ruby/ql/lib/codeql/ruby/ApiGraphs.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,7 @@ module API {
203203
/**
204204
* Gets a node representing a call to `method` on the receiver represented by this node.
205205
*/
206-
Node getMethod(string method) {
206+
MethodAccessNode getMethod(string method) {
207207
result = this.getASubclass().getASuccessor(Label::method(method))
208208
}
209209

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

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -581,3 +581,49 @@ private class CipherOperation extends Cryptography::CryptographicOperation::Rang
581581
result = cipherNode.getCipherMode().getBlockMode()
582582
}
583583
}
584+
585+
/** Predicates and classes modelling the `OpenSSL::Digest` module */
586+
private module Digest {
587+
private import codeql.ruby.ApiGraphs
588+
589+
/** A call that hashes some input using a hashing algorithm from the `OpenSSL::Digest` module. */
590+
private class DigestCall extends Cryptography::CryptographicOperation::Range instanceof DataFlow::CallNode {
591+
Cryptography::HashingAlgorithm algo;
592+
593+
DigestCall() {
594+
exists(API::MethodAccessNode call |
595+
call = API::getTopLevelMember("OpenSSL").getMember("Digest").getMethod("new")
596+
|
597+
this = call.getReturn().getAMethodCall(["digest", "update", "<<"]) and
598+
algo.matchesName(call.getCallNode()
599+
.getArgument(0)
600+
.asExpr()
601+
.getExpr()
602+
.getConstantValue()
603+
.getString())
604+
)
605+
}
606+
607+
override Cryptography::HashingAlgorithm getAlgorithm() { result = algo }
608+
609+
override DataFlow::Node getAnInput() { result = super.getArgument(0) }
610+
611+
override Cryptography::BlockMode getBlockMode() { none() }
612+
}
613+
614+
/** A call to `OpenSSL::Digest.digest` that hashes input directly without constructing a digest instance. */
615+
private class DigestCallDirect extends Cryptography::CryptographicOperation::Range instanceof DataFlow::CallNode {
616+
Cryptography::HashingAlgorithm algo;
617+
618+
DigestCallDirect() {
619+
this = API::getTopLevelMember("OpenSSL").getMember("Digest").getMethod("digest").getCallNode() and
620+
algo.matchesName(this.getArgument(0).asExpr().getExpr().getConstantValue().getString())
621+
}
622+
623+
override Cryptography::HashingAlgorithm getAlgorithm() { result = algo }
624+
625+
override DataFlow::Node getAnInput() { result = super.getArgument(1) }
626+
627+
override Cryptography::BlockMode getBlockMode() { none() }
628+
}
629+
}

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,3 +24,6 @@
2424
| broken_crypto.rb:90:1:90:17 | ... << ... | The cryptographic algorithm MD5 is broken or weak, and should not be used. |
2525
| broken_crypto.rb:95:1:95:34 | call to bubblebabble | The cryptographic algorithm MD5 is broken or weak, and should not be used. |
2626
| broken_crypto.rb:97:11:97:37 | call to file | The cryptographic algorithm MD5 is broken or weak, and should not be used. |
27+
| broken_crypto.rb:103:1:103:21 | call to digest | The cryptographic algorithm SHA1 is broken or weak, and should not be used. |
28+
| broken_crypto.rb:104:1:104:17 | ... << ... | The cryptographic algorithm SHA1 is broken or weak, and should not be used. |
29+
| broken_crypto.rb:106:1:106:37 | call to digest | The cryptographic algorithm SHA1 is broken or weak, and should not be used. |

ruby/ql/test/query-tests/security/cwe-327/broken_crypto.rb

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,4 +97,11 @@
9797
filemd5 = Digest::MD5.file 'testfile'
9898
filemd5.hexdigest
9999

100-
Digest("MD5").hexdigest('foo') # BAD: weak hash algorithm
100+
Digest("MD5").hexdigest('foo') # BAD: weak hash algorithm
101+
102+
sha1 = OpenSSL::Digest.new('SHA1')
103+
sha1.digest 'message' # BAD: weak hash algorithm
104+
sha1 << 'message' # << is an alias for update
105+
106+
OpenSSL::Digest.digest('SHA1', "abc") # BAD: weak hash algorithm
107+
OpenSSL::Digest.digest('SHA3-512', "abc") # GOOD: strong hash algorithm

0 commit comments

Comments
 (0)