diff --git a/ruby/ql/src/experimental/cwe-208/UnsafeHmacComparison.ql b/ruby/ql/src/experimental/cwe-208/UnsafeHmacComparison.ql new file mode 100644 index 000000000000..65907b0f8859 --- /dev/null +++ b/ruby/ql/src/experimental/cwe-208/UnsafeHmacComparison.ql @@ -0,0 +1,45 @@ +/** + * @name Unsafe HMAC Comparison + * @description An HMAC is being compared using the equality operator. This may be vulnerable to a cryptographic timing attack + * because the equality operation does not occur in constant time." + * @kind path-problem + * @problem.severity error + * @security-severity 6.0 + * @precision high + * @id rb/unsafe-hmac-comparison + * @tags security + * external/cwe/cwe-208 + */ + +private import codeql.ruby.AST +private import codeql.ruby.DataFlow +private import codeql.ruby.ApiGraphs + +private class OpenSslHmacSource extends DataFlow::Node { + OpenSslHmacSource() { + exists(API::Node hmacNode | hmacNode = API::getTopLevelMember("OpenSSL").getMember("HMAC") | + this = hmacNode.getAMethodCall(["hexdigest", "to_s", "digest", "base64digest"]) + or + this = hmacNode.getAnInstantiation() + ) + } +} + +private module UnsafeHmacComparison { + private module Config implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { source instanceof OpenSslHmacSource } + + // Holds if a given sink is an Equality Operation (== or !=) + predicate isSink(DataFlow::Node sink) { + any(EqualityOperation eqOp).getAnOperand() = sink.asExpr().getExpr() + } + } + + import DataFlow::Global +} + +private import UnsafeHmacComparison::PathGraph + +from UnsafeHmacComparison::PathNode source, UnsafeHmacComparison::PathNode sink +where UnsafeHmacComparison::flowPath(source, sink) +select sink.getNode(), source, sink, "This comparison is potentially vulnerable to a timing attack." diff --git a/ruby/ql/src/experimental/cwe-208/UnsafeHmacComparison.qlhelp b/ruby/ql/src/experimental/cwe-208/UnsafeHmacComparison.qlhelp new file mode 100644 index 000000000000..5c589466cc18 --- /dev/null +++ b/ruby/ql/src/experimental/cwe-208/UnsafeHmacComparison.qlhelp @@ -0,0 +1,21 @@ + + + +

+ Using the `==` or `!=` operator to compare a known valid HMAC with a user-supplied HMAC digest could lead to a timing attack, as these operations do not occur in constant time. +

+
+ +

+ Instead of using `==` or `!=` to compare a known valid HMAC with a user-supplied HMAC digest use Rack::Utils#secure_compare, ActiveSupport::SecurityUtils#secure_compare or OpenSSL.secure_compare +

+
+ +

+ In this example, the HMAC is validated using the `==` operation. +

+ +
+
diff --git a/ruby/ql/src/experimental/cwe-208/examples/unsafe_hmac_comparison.rb b/ruby/ql/src/experimental/cwe-208/examples/unsafe_hmac_comparison.rb new file mode 100644 index 000000000000..b3b07012a372 --- /dev/null +++ b/ruby/ql/src/experimental/cwe-208/examples/unsafe_hmac_comparison.rb @@ -0,0 +1,11 @@ +class UnsafeHmacComparison + def verify_hmac(host, hmac, salt) + sha1 = OpenSSL::Digest.new('sha1') + if OpenSSL::HMAC.digest(sha1, salt, host) == hmac + puts "HMAC verified" + else + puts "HMAC not verified" + end + end + end + \ No newline at end of file