From 494e7d9a3f16321a4764074d5bd96774c5d0fa35 Mon Sep 17 00:00:00 2001 From: Brandon Stewart <20469703+boveus@users.noreply.github.com> Date: Wed, 26 Jul 2023 17:28:22 +0000 Subject: [PATCH 01/14] add unsafe HMAC comparison query and qlhelp file --- .../cwe-208/UnsafeHMACComparison.ql | 73 +++++++++++++++++++ .../cwe-208/UnsafeHMACComparison.qlhelp | 21 ++++++ .../examples/unsafe_hmac_comparison.rb | 11 +++ 3 files changed, 105 insertions(+) create mode 100644 ruby/ql/src/experimental/cwe-208/UnsafeHMACComparison.ql create mode 100644 ruby/ql/src/experimental/cwe-208/UnsafeHMACComparison.qlhelp create mode 100644 ruby/ql/src/experimental/cwe-208/examples/unsafe_hmac_comparison.rb 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..27562174f0a0 --- /dev/null +++ b/ruby/ql/src/experimental/cwe-208/UnsafeHMACComparison.ql @@ -0,0 +1,73 @@ +private import codeql.ruby.AST +import codeql.ruby.AST +import codeql.ruby.DataFlow +import codeql.ruby.ApiGraphs +import codeql.ruby.dataflow.RemoteFlowSources +import codeql.ruby.ast.Operation +import codeql.ruby.TaintTracking +import ruby + +/** + * @kind problem + */ + +// A call to OpenSSL::HMAC.hexdigest +class OpenSSLHMACHexdigest extends DataFlow::Node { + OpenSSLHMACHexdigest() { + this = API::getTopLevelMember("OpenSSL").getMember("HMAC").getAMethodCall("hexdigest") + } +} + +// A call to OpenSSL::HMAC.to_s (which is an alias for OpenSSL::HMAC.hexdigest) +class OpenSSLHMACtos extends DataFlow::Node { + OpenSSLHMACtos() { + this = API::getTopLevelMember("OpenSSL").getMember("HMAC").getAMethodCall("to_s") + } +} + +// A call to OpenSSL::HMAC.digest +class OpenSSLHMACdigest extends DataFlow::Node { + OpenSSLHMACdigest() { + this = API::getTopLevelMember("OpenSSL").getMember("HMAC").getAMethodCall("digest") + } +} + +// A call to OpenSSL::HMAC.new +class OpenSSLnewHMAC extends DataFlow::Node { + OpenSSLnewHMAC() { + this = API::getTopLevelMember("OpenSSL").getMember("HMAC").getAnInstantiation() + } +} + +// A call to OpenSSL::HMAC.base64digest +class OpenSSLHmacbase64digest extends DataFlow::Node { + OpenSSLHmacbase64digest() { + this = API::getTopLevelMember("OpenSSL").getMember("HMAC").getAMethodCall("base64digest") + } +} + +class Configuration extends DataFlow::Configuration { + Configuration() { this = "UnsafeHMACComparison" } + + override predicate isSource(DataFlow::Node source) { + source instanceof OpenSSLHMACHexdigest or + source instanceof OpenSSLnewHMAC or + source instanceof OpenSSLHmacbase64digest or + source instanceof OpenSSLHMACdigest or + source instanceof OpenSSLHMACtos + } + + // Holds if a given sink is an Equality Operation (== or !=) + override predicate isSink(DataFlow::Node sink) { + exists(EqualityOperation eqOp | + eqOp.getLeftOperand() = sink.asExpr().getExpr() + or + eqOp.getRightOperand() = sink.asExpr().getExpr() + ) + } +} + +from DataFlow::Node source, DataFlow::Node sink, Configuration config +where config.hasFlow(source, sink) +select sink, + "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." 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 From adddc58b6126de7d0f1d34cbaec3977e643dea98 Mon Sep 17 00:00:00 2001 From: Brandon Stewart <20469703+boveus@users.noreply.github.com> Date: Wed, 26 Jul 2023 17:38:06 +0000 Subject: [PATCH 02/14] address linter --- ...CComparison.ql => UnsafeHmacComparison.ql} | 29 ++++++++----------- ...son.qlhelp => UnsafeHmacComparison.qlhelp} | 0 2 files changed, 12 insertions(+), 17 deletions(-) rename ruby/ql/src/experimental/cwe-208/{UnsafeHMACComparison.ql => UnsafeHmacComparison.ql} (76%) rename ruby/ql/src/experimental/cwe-208/{UnsafeHMACComparison.qlhelp => UnsafeHmacComparison.qlhelp} (100%) diff --git a/ruby/ql/src/experimental/cwe-208/UnsafeHMACComparison.ql b/ruby/ql/src/experimental/cwe-208/UnsafeHmacComparison.ql similarity index 76% rename from ruby/ql/src/experimental/cwe-208/UnsafeHMACComparison.ql rename to ruby/ql/src/experimental/cwe-208/UnsafeHmacComparison.ql index 27562174f0a0..e8749159d28d 100644 --- a/ruby/ql/src/experimental/cwe-208/UnsafeHMACComparison.ql +++ b/ruby/ql/src/experimental/cwe-208/UnsafeHmacComparison.ql @@ -3,31 +3,26 @@ import codeql.ruby.AST import codeql.ruby.DataFlow import codeql.ruby.ApiGraphs import codeql.ruby.dataflow.RemoteFlowSources -import codeql.ruby.ast.Operation import codeql.ruby.TaintTracking import ruby -/** - * @kind problem - */ - // A call to OpenSSL::HMAC.hexdigest -class OpenSSLHMACHexdigest extends DataFlow::Node { - OpenSSLHMACHexdigest() { +class OpenSslHmacHexdigest extends DataFlow::Node { + OpenSslHmacHexdigest() { this = API::getTopLevelMember("OpenSSL").getMember("HMAC").getAMethodCall("hexdigest") } } // A call to OpenSSL::HMAC.to_s (which is an alias for OpenSSL::HMAC.hexdigest) -class OpenSSLHMACtos extends DataFlow::Node { - OpenSSLHMACtos() { +class OpenSslHmactos extends DataFlow::Node { + OpenSslHmactos() { this = API::getTopLevelMember("OpenSSL").getMember("HMAC").getAMethodCall("to_s") } } // A call to OpenSSL::HMAC.digest -class OpenSSLHMACdigest extends DataFlow::Node { - OpenSSLHMACdigest() { +class OpenSslHmacdigest extends DataFlow::Node { + OpenSslHmacdigest() { this = API::getTopLevelMember("OpenSSL").getMember("HMAC").getAMethodCall("digest") } } @@ -40,8 +35,8 @@ class OpenSSLnewHMAC extends DataFlow::Node { } // A call to OpenSSL::HMAC.base64digest -class OpenSSLHmacbase64digest extends DataFlow::Node { - OpenSSLHmacbase64digest() { +class OpenSslHmacbase64digest extends DataFlow::Node { + OpenSslHmacbase64digest() { this = API::getTopLevelMember("OpenSSL").getMember("HMAC").getAMethodCall("base64digest") } } @@ -50,11 +45,11 @@ class Configuration extends DataFlow::Configuration { Configuration() { this = "UnsafeHMACComparison" } override predicate isSource(DataFlow::Node source) { - source instanceof OpenSSLHMACHexdigest or + source instanceof OpenSslHmacHexdigest or source instanceof OpenSSLnewHMAC or - source instanceof OpenSSLHmacbase64digest or - source instanceof OpenSSLHMACdigest or - source instanceof OpenSSLHMACtos + source instanceof OpenSslHmacbase64digest or + source instanceof OpenSslHmacdigest or + source instanceof OpenSslHmactos } // Holds if a given sink is an Equality Operation (== or !=) diff --git a/ruby/ql/src/experimental/cwe-208/UnsafeHMACComparison.qlhelp b/ruby/ql/src/experimental/cwe-208/UnsafeHmacComparison.qlhelp similarity index 100% rename from ruby/ql/src/experimental/cwe-208/UnsafeHMACComparison.qlhelp rename to ruby/ql/src/experimental/cwe-208/UnsafeHmacComparison.qlhelp From 42adbe0cd4f2749941dfedce6305e9852df26506 Mon Sep 17 00:00:00 2001 From: Brandon Stewart <20469703+boveus@users.noreply.github.com> Date: Wed, 26 Jul 2023 17:43:34 +0000 Subject: [PATCH 03/14] address linter --- ruby/ql/src/experimental/cwe-208/UnsafeHmacComparison.ql | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ruby/ql/src/experimental/cwe-208/UnsafeHmacComparison.ql b/ruby/ql/src/experimental/cwe-208/UnsafeHmacComparison.ql index e8749159d28d..4192b617863f 100644 --- a/ruby/ql/src/experimental/cwe-208/UnsafeHmacComparison.ql +++ b/ruby/ql/src/experimental/cwe-208/UnsafeHmacComparison.ql @@ -28,8 +28,8 @@ class OpenSslHmacdigest extends DataFlow::Node { } // A call to OpenSSL::HMAC.new -class OpenSSLnewHMAC extends DataFlow::Node { - OpenSSLnewHMAC() { +class OpenSsslnewHmac extends DataFlow::Node { + OpenSsslnewHmac() { this = API::getTopLevelMember("OpenSSL").getMember("HMAC").getAnInstantiation() } } @@ -46,7 +46,7 @@ class Configuration extends DataFlow::Configuration { override predicate isSource(DataFlow::Node source) { source instanceof OpenSslHmacHexdigest or - source instanceof OpenSSLnewHMAC or + source instanceof OpenSsslnewHmac or source instanceof OpenSslHmacbase64digest or source instanceof OpenSslHmacdigest or source instanceof OpenSslHmactos From 346a2f269eedda90093c19f314dd3af1fd58d1fc Mon Sep 17 00:00:00 2001 From: Brandon Stewart <20469703+boveus@users.noreply.github.com> Date: Wed, 26 Jul 2023 13:48:42 -0400 Subject: [PATCH 04/14] Update UnsafeHmacComparison.ql --- ruby/ql/src/experimental/cwe-208/UnsafeHmacComparison.ql | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ruby/ql/src/experimental/cwe-208/UnsafeHmacComparison.ql b/ruby/ql/src/experimental/cwe-208/UnsafeHmacComparison.ql index 4192b617863f..3efed3580aa3 100644 --- a/ruby/ql/src/experimental/cwe-208/UnsafeHmacComparison.ql +++ b/ruby/ql/src/experimental/cwe-208/UnsafeHmacComparison.ql @@ -28,7 +28,7 @@ class OpenSslHmacdigest extends DataFlow::Node { } // A call to OpenSSL::HMAC.new -class OpenSsslnewHmac extends DataFlow::Node { +class OpenSslnewHmac extends DataFlow::Node { OpenSsslnewHmac() { this = API::getTopLevelMember("OpenSSL").getMember("HMAC").getAnInstantiation() } @@ -46,7 +46,7 @@ class Configuration extends DataFlow::Configuration { override predicate isSource(DataFlow::Node source) { source instanceof OpenSslHmacHexdigest or - source instanceof OpenSsslnewHmac or + source instanceof OpenSslnewHmac or source instanceof OpenSslHmacbase64digest or source instanceof OpenSslHmacdigest or source instanceof OpenSslHmactos From 1a83554b0c7d2daec8228da5838342968ff616fe Mon Sep 17 00:00:00 2001 From: Brandon Stewart <20469703+boveus@users.noreply.github.com> Date: Wed, 26 Jul 2023 17:54:42 +0000 Subject: [PATCH 05/14] correct typo --- ruby/ql/src/experimental/cwe-208/UnsafeHmacComparison.ql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ruby/ql/src/experimental/cwe-208/UnsafeHmacComparison.ql b/ruby/ql/src/experimental/cwe-208/UnsafeHmacComparison.ql index 3efed3580aa3..b84f2f866489 100644 --- a/ruby/ql/src/experimental/cwe-208/UnsafeHmacComparison.ql +++ b/ruby/ql/src/experimental/cwe-208/UnsafeHmacComparison.ql @@ -29,7 +29,7 @@ class OpenSslHmacdigest extends DataFlow::Node { // A call to OpenSSL::HMAC.new class OpenSslnewHmac extends DataFlow::Node { - OpenSsslnewHmac() { + OpenSslnewHmac() { this = API::getTopLevelMember("OpenSSL").getMember("HMAC").getAnInstantiation() } } From f241498cab63d0df79e9f42a67bc9f6892c721e6 Mon Sep 17 00:00:00 2001 From: Brandon Stewart <20469703+boveus@users.noreply.github.com> Date: Wed, 26 Jul 2023 17:55:56 +0000 Subject: [PATCH 06/14] correct additional pascalcase issue --- ruby/ql/src/experimental/cwe-208/UnsafeHmacComparison.ql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ruby/ql/src/experimental/cwe-208/UnsafeHmacComparison.ql b/ruby/ql/src/experimental/cwe-208/UnsafeHmacComparison.ql index b84f2f866489..253f78840fb1 100644 --- a/ruby/ql/src/experimental/cwe-208/UnsafeHmacComparison.ql +++ b/ruby/ql/src/experimental/cwe-208/UnsafeHmacComparison.ql @@ -42,7 +42,7 @@ class OpenSslHmacbase64digest extends DataFlow::Node { } class Configuration extends DataFlow::Configuration { - Configuration() { this = "UnsafeHMACComparison" } + Configuration() { this = "UnsafeHmacComparison" } override predicate isSource(DataFlow::Node source) { source instanceof OpenSslHmacHexdigest or From 93dd9d0aa4d6a1d23599e93fe9e3f13e2c0d797d Mon Sep 17 00:00:00 2001 From: Brandon Stewart <20469703+boveus@users.noreply.github.com> Date: Tue, 8 Aug 2023 12:54:54 -0400 Subject: [PATCH 07/14] Update ruby/ql/src/experimental/cwe-208/UnsafeHmacComparison.ql Co-authored-by: Alex Ford --- ruby/ql/src/experimental/cwe-208/UnsafeHmacComparison.ql | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/ruby/ql/src/experimental/cwe-208/UnsafeHmacComparison.ql b/ruby/ql/src/experimental/cwe-208/UnsafeHmacComparison.ql index 253f78840fb1..1893eb94ae55 100644 --- a/ruby/ql/src/experimental/cwe-208/UnsafeHmacComparison.ql +++ b/ruby/ql/src/experimental/cwe-208/UnsafeHmacComparison.ql @@ -54,11 +54,7 @@ class Configuration extends DataFlow::Configuration { // Holds if a given sink is an Equality Operation (== or !=) override predicate isSink(DataFlow::Node sink) { - exists(EqualityOperation eqOp | - eqOp.getLeftOperand() = sink.asExpr().getExpr() - or - eqOp.getRightOperand() = sink.asExpr().getExpr() - ) + any(EqualityOperation eqOp).getAnOperand() = sink.asExpr().getExpr() } } From 26401fec7076cb6556ec9e8a21df09eb493a3b6a Mon Sep 17 00:00:00 2001 From: Brandon Stewart <20469703+boveus@users.noreply.github.com> Date: Wed, 9 Aug 2023 18:44:42 +0000 Subject: [PATCH 08/14] address PR comments --- .../cwe-208/UnsafeHmacComparison.ql | 87 ++++++++----------- 1 file changed, 35 insertions(+), 52 deletions(-) diff --git a/ruby/ql/src/experimental/cwe-208/UnsafeHmacComparison.ql b/ruby/ql/src/experimental/cwe-208/UnsafeHmacComparison.ql index 1893eb94ae55..8210eeb3c665 100644 --- a/ruby/ql/src/experimental/cwe-208/UnsafeHmacComparison.ql +++ b/ruby/ql/src/experimental/cwe-208/UnsafeHmacComparison.ql @@ -1,64 +1,47 @@ -private import codeql.ruby.AST -import codeql.ruby.AST -import codeql.ruby.DataFlow +/** + * @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.DataFlow import codeql.ruby.ApiGraphs -import codeql.ruby.dataflow.RemoteFlowSources -import codeql.ruby.TaintTracking import ruby -// A call to OpenSSL::HMAC.hexdigest -class OpenSslHmacHexdigest extends DataFlow::Node { - OpenSslHmacHexdigest() { - this = API::getTopLevelMember("OpenSSL").getMember("HMAC").getAMethodCall("hexdigest") +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() + ) } } -// A call to OpenSSL::HMAC.to_s (which is an alias for OpenSSL::HMAC.hexdigest) -class OpenSslHmactos extends DataFlow::Node { - OpenSslHmactos() { - this = API::getTopLevelMember("OpenSSL").getMember("HMAC").getAMethodCall("to_s") - } -} - -// A call to OpenSSL::HMAC.digest -class OpenSslHmacdigest extends DataFlow::Node { - OpenSslHmacdigest() { - this = API::getTopLevelMember("OpenSSL").getMember("HMAC").getAMethodCall("digest") - } -} - -// A call to OpenSSL::HMAC.new -class OpenSslnewHmac extends DataFlow::Node { - OpenSslnewHmac() { - this = API::getTopLevelMember("OpenSSL").getMember("HMAC").getAnInstantiation() - } -} - -// A call to OpenSSL::HMAC.base64digest -class OpenSslHmacbase64digest extends DataFlow::Node { - OpenSslHmacbase64digest() { - this = API::getTopLevelMember("OpenSSL").getMember("HMAC").getAMethodCall("base64digest") - } -} - -class Configuration extends DataFlow::Configuration { - Configuration() { this = "UnsafeHmacComparison" } - - override predicate isSource(DataFlow::Node source) { - source instanceof OpenSslHmacHexdigest or - source instanceof OpenSslnewHmac or - source instanceof OpenSslHmacbase64digest or - source instanceof OpenSslHmacdigest or - source instanceof OpenSslHmactos - } +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 !=) - override predicate isSink(DataFlow::Node sink) { + predicate isSink(DataFlow::Node sink) { any(EqualityOperation eqOp).getAnOperand() = sink.asExpr().getExpr() } + } + + import DataFlow::Global } -from DataFlow::Node source, DataFlow::Node sink, Configuration config -where config.hasFlow(source, sink) -select sink, - "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." +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." \ No newline at end of file From 07d5beca343727db38486fb930d9d018d33f0693 Mon Sep 17 00:00:00 2001 From: Brandon Stewart <20469703+boveus@users.noreply.github.com> Date: Wed, 9 Aug 2023 18:51:55 +0000 Subject: [PATCH 09/14] run format document --- .../cwe-208/UnsafeHmacComparison.ql | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/ruby/ql/src/experimental/cwe-208/UnsafeHmacComparison.ql b/ruby/ql/src/experimental/cwe-208/UnsafeHmacComparison.ql index 8210eeb3c665..48f379ddcdc9 100644 --- a/ruby/ql/src/experimental/cwe-208/UnsafeHmacComparison.ql +++ b/ruby/ql/src/experimental/cwe-208/UnsafeHmacComparison.ql @@ -2,7 +2,6 @@ * @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 @@ -12,7 +11,7 @@ * external/cwe/cwe-208 */ - +private import codeql.ruby.AST private import codeql.ruby.DataFlow import codeql.ruby.ApiGraphs import ruby @@ -29,14 +28,12 @@ private class OpenSslHmacSource extends DataFlow::Node { private module UnsafeHmacComparison { private module Config implements DataFlow::ConfigSig { - predicate isSource(DataFlow::Node source) { - source instanceof OpenSslHmacSource - } + 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() - } + // 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 @@ -44,4 +41,4 @@ private module UnsafeHmacComparison { 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." \ No newline at end of file +select sink.getNode(), source, sink, "This comparison is potentially vulnerable to a timing attack." From cca4c35cf8326c84fba56ea74a46f578af7e36e9 Mon Sep 17 00:00:00 2001 From: Brandon Stewart <20469703+boveus@users.noreply.github.com> Date: Wed, 9 Aug 2023 19:23:21 +0000 Subject: [PATCH 10/14] add pathgraph --- ruby/ql/src/experimental/cwe-208/UnsafeHmacComparison.ql | 1 + 1 file changed, 1 insertion(+) diff --git a/ruby/ql/src/experimental/cwe-208/UnsafeHmacComparison.ql b/ruby/ql/src/experimental/cwe-208/UnsafeHmacComparison.ql index 48f379ddcdc9..7dace03c671f 100644 --- a/ruby/ql/src/experimental/cwe-208/UnsafeHmacComparison.ql +++ b/ruby/ql/src/experimental/cwe-208/UnsafeHmacComparison.ql @@ -13,6 +13,7 @@ private import codeql.ruby.AST private import codeql.ruby.DataFlow +import DataFlow::PathGraph import codeql.ruby.ApiGraphs import ruby From 74567041a74b3ff0331349a98b5472fbbd7be36c Mon Sep 17 00:00:00 2001 From: Brandon Stewart <20469703+boveus@users.noreply.github.com> Date: Wed, 9 Aug 2023 19:51:07 +0000 Subject: [PATCH 11/14] remove pathgraph --- ruby/ql/src/experimental/cwe-208/UnsafeHmacComparison.ql | 1 - 1 file changed, 1 deletion(-) diff --git a/ruby/ql/src/experimental/cwe-208/UnsafeHmacComparison.ql b/ruby/ql/src/experimental/cwe-208/UnsafeHmacComparison.ql index 7dace03c671f..48f379ddcdc9 100644 --- a/ruby/ql/src/experimental/cwe-208/UnsafeHmacComparison.ql +++ b/ruby/ql/src/experimental/cwe-208/UnsafeHmacComparison.ql @@ -13,7 +13,6 @@ private import codeql.ruby.AST private import codeql.ruby.DataFlow -import DataFlow::PathGraph import codeql.ruby.ApiGraphs import ruby From 7882cf0bf0e93c99713fe1d8b01324cf5c8e4ca9 Mon Sep 17 00:00:00 2001 From: Brandon Stewart <20469703+boveus@users.noreply.github.com> Date: Thu, 10 Aug 2023 09:21:02 -0400 Subject: [PATCH 12/14] Update ruby/ql/src/experimental/cwe-208/UnsafeHmacComparison.ql Co-authored-by: Alex Ford --- ruby/ql/src/experimental/cwe-208/UnsafeHmacComparison.ql | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/ruby/ql/src/experimental/cwe-208/UnsafeHmacComparison.ql b/ruby/ql/src/experimental/cwe-208/UnsafeHmacComparison.ql index 48f379ddcdc9..9c7de85ebc88 100644 --- a/ruby/ql/src/experimental/cwe-208/UnsafeHmacComparison.ql +++ b/ruby/ql/src/experimental/cwe-208/UnsafeHmacComparison.ql @@ -13,8 +13,7 @@ private import codeql.ruby.AST private import codeql.ruby.DataFlow -import codeql.ruby.ApiGraphs -import ruby +private import codeql.ruby.ApiGraphs private class OpenSslHmacSource extends DataFlow::Node { OpenSslHmacSource() { From b899b648e5a24b536d3bedea69c5c37d2977b2dc Mon Sep 17 00:00:00 2001 From: Brandon Stewart <20469703+boveus@users.noreply.github.com> Date: Thu, 10 Aug 2023 09:21:16 -0400 Subject: [PATCH 13/14] Update ruby/ql/src/experimental/cwe-208/UnsafeHmacComparison.ql Co-authored-by: Alex Ford --- ruby/ql/src/experimental/cwe-208/UnsafeHmacComparison.ql | 1 + 1 file changed, 1 insertion(+) diff --git a/ruby/ql/src/experimental/cwe-208/UnsafeHmacComparison.ql b/ruby/ql/src/experimental/cwe-208/UnsafeHmacComparison.ql index 9c7de85ebc88..e32c534d2130 100644 --- a/ruby/ql/src/experimental/cwe-208/UnsafeHmacComparison.ql +++ b/ruby/ql/src/experimental/cwe-208/UnsafeHmacComparison.ql @@ -38,6 +38,7 @@ private module UnsafeHmacComparison { 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." From 01577dac3221a205ec4e82084fc25c4ec20f4de7 Mon Sep 17 00:00:00 2001 From: Brandon Stewart <20469703+boveus@users.noreply.github.com> Date: Thu, 10 Aug 2023 13:59:47 +0000 Subject: [PATCH 14/14] format document --- ruby/ql/src/experimental/cwe-208/UnsafeHmacComparison.ql | 1 + 1 file changed, 1 insertion(+) diff --git a/ruby/ql/src/experimental/cwe-208/UnsafeHmacComparison.ql b/ruby/ql/src/experimental/cwe-208/UnsafeHmacComparison.ql index e32c534d2130..65907b0f8859 100644 --- a/ruby/ql/src/experimental/cwe-208/UnsafeHmacComparison.ql +++ b/ruby/ql/src/experimental/cwe-208/UnsafeHmacComparison.ql @@ -39,6 +39,7 @@ private module UnsafeHmacComparison { } 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."