From 7e1c57689bc79dc91e2755dd0559f6c03f8751a2 Mon Sep 17 00:00:00 2001 From: edvraa <80588099+edvraa@users.noreply.github.com> Date: Mon, 26 Apr 2021 12:34:55 +0300 Subject: [PATCH 1/4] Insufficient key size --- .../CWE-326/InsufficientKeySize.qhelp | 50 +++++++++++++++++++ .../CWE-326/InsufficientKeySize.ql | 35 +++++++++++++ .../CWE-326/InsufficientKeySizeBad.go | 16 ++++++ .../CWE-326/InsufficientKeySizeGood.go | 16 ++++++ .../CWE-326/InsufficientKeySize.expected | 15 ++++++ .../CWE-326/InsufficientKeySize.go | 34 +++++++++++++ .../CWE-326/InsufficientKeySize.qlref | 1 + 7 files changed, 167 insertions(+) create mode 100644 ql/src/experimental/CWE-326/InsufficientKeySize.qhelp create mode 100644 ql/src/experimental/CWE-326/InsufficientKeySize.ql create mode 100644 ql/src/experimental/CWE-326/InsufficientKeySizeBad.go create mode 100644 ql/src/experimental/CWE-326/InsufficientKeySizeGood.go create mode 100644 ql/test/experimental/CWE-326/InsufficientKeySize.expected create mode 100644 ql/test/experimental/CWE-326/InsufficientKeySize.go create mode 100644 ql/test/experimental/CWE-326/InsufficientKeySize.qlref diff --git a/ql/src/experimental/CWE-326/InsufficientKeySize.qhelp b/ql/src/experimental/CWE-326/InsufficientKeySize.qhelp new file mode 100644 index 00000000000..c18341ca922 --- /dev/null +++ b/ql/src/experimental/CWE-326/InsufficientKeySize.qhelp @@ -0,0 +1,50 @@ + + + +

+ Incorrect uses of encryption algorithms may result in sensitive data exposure, + key leakage, broken authentication, insecure session, and spoofing attacks. +

+ + + + +

+ Ensure that you use a strong key with a recommended bit size. + For RSA encryption the minimum size is 2048 bits. +

+ + + + +

+ The following code uses RSA encryption with insufficient key size. +

+ + + +

+ In the example below the key size is set to 2048 bits. +

+ + + + + + +
  • OWASP: Cryptographic Storage Cheat Sheet. +
    • +
  • Wikipedia: Cryptographically Strong Algorithms. +
    • +
  • Wikipedia: Strong Cryptography Examples. +
    • +
  • NIST, FIPS 140 Annex a: Approved Security Functions.
    • +
  • NIST, SP 800-131A: Transitions: Recommendation for Transitioning the Use of Cryptographic Algorithms and Key Lengths.
    • +     + +     diff --git a/ql/src/experimental/CWE-326/InsufficientKeySize.ql b/ql/src/experimental/CWE-326/InsufficientKeySize.ql new file mode 100644 index 00000000000..c42def27a4c --- /dev/null +++ b/ql/src/experimental/CWE-326/InsufficientKeySize.ql @@ -0,0 +1,35 @@ +/** + * @name Use of a weak cryptographic key + * @description Using weak cryptographic key can allow an attacker to compromise security. + * @kind path-problem + * @problem.severity error + * @id go/weak-crypto-key + * @tags security + * external/cwe/cwe-326 + */ + +import go +import DataFlow::PathGraph + +class RsaKeyTrackingConfiguration extends DataFlow::Configuration { + RsaKeyTrackingConfiguration() { this = "RsaKeyTrackingConfiguration" } + + override predicate isSource(DataFlow::Node source) { + exists(ValueExpr c | + source.asExpr() = c and + c.getIntValue() < 2048 + ) + } + + override predicate isSink(DataFlow::Node sink) { + exists(CallExpr c | + sink.asExpr() = c.getArgument(1) and + c.getTarget().hasQualifiedName("crypto/rsa", "GenerateKey") + ) + } +} + +from RsaKeyTrackingConfiguration cfg, DataFlow::PathNode source, DataFlow::PathNode sink +where cfg.hasFlowPath(source, sink) +select sink, source, sink, "The size of RSA key '$@' should be at least 2048 bits.", sink, + source.getNode().toString() diff --git a/ql/src/experimental/CWE-326/InsufficientKeySizeBad.go b/ql/src/experimental/CWE-326/InsufficientKeySizeBad.go new file mode 100644 index 00000000000..73abf362758 --- /dev/null +++ b/ql/src/experimental/CWE-326/InsufficientKeySizeBad.go @@ -0,0 +1,16 @@ +package main + +import ( + "crypto/rand" + "crypto/rsa" + "fmt" +) + +func main() { + //Generate Private Key + pvk, err := rsa.GenerateKey(rand.Reader, 1024) + if err != nil { + fmt.Println(err) + } + fmt.Println(pvk) +} \ No newline at end of file diff --git a/ql/src/experimental/CWE-326/InsufficientKeySizeGood.go b/ql/src/experimental/CWE-326/InsufficientKeySizeGood.go new file mode 100644 index 00000000000..f72efecd5c6 --- /dev/null +++ b/ql/src/experimental/CWE-326/InsufficientKeySizeGood.go @@ -0,0 +1,16 @@ +package main + +import ( + "crypto/rand" + "crypto/rsa" + "fmt" +) + +func main() { + //Generate Private Key + pvk, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + fmt.Println(err) + } + fmt.Println(pvk) +} \ No newline at end of file diff --git a/ql/test/experimental/CWE-326/InsufficientKeySize.expected b/ql/test/experimental/CWE-326/InsufficientKeySize.expected new file mode 100644 index 00000000000..d02a211a172 --- /dev/null +++ b/ql/test/experimental/CWE-326/InsufficientKeySize.expected @@ -0,0 +1,15 @@ +edges +| InsufficientKeySize.go:13:10:13:13 | 1024 : int | InsufficientKeySize.go:14:31:14:34 | size | +| InsufficientKeySize.go:18:7:18:10 | 1024 : int | InsufficientKeySize.go:25:11:25:14 | definition of size : int | +| InsufficientKeySize.go:25:11:25:14 | definition of size : int | InsufficientKeySize.go:26:31:26:34 | size | +nodes +| InsufficientKeySize.go:9:31:9:34 | 1024 | semmle.label | 1024 | +| InsufficientKeySize.go:13:10:13:13 | 1024 : int | semmle.label | 1024 : int | +| InsufficientKeySize.go:14:31:14:34 | size | semmle.label | size | +| InsufficientKeySize.go:18:7:18:10 | 1024 : int | semmle.label | 1024 : int | +| InsufficientKeySize.go:25:11:25:14 | definition of size : int | semmle.label | definition of size : int | +| InsufficientKeySize.go:26:31:26:34 | size | semmle.label | size | +#select +| InsufficientKeySize.go:9:31:9:34 | 1024 | InsufficientKeySize.go:9:31:9:34 | 1024 | InsufficientKeySize.go:9:31:9:34 | 1024 | The size of RSA key '$@' should be at least 2048 bits. | InsufficientKeySize.go:9:31:9:34 | 1024 | 1024 | +| InsufficientKeySize.go:14:31:14:34 | size | InsufficientKeySize.go:13:10:13:13 | 1024 : int | InsufficientKeySize.go:14:31:14:34 | size | The size of RSA key '$@' should be at least 2048 bits. | InsufficientKeySize.go:14:31:14:34 | size | 1024 | +| InsufficientKeySize.go:26:31:26:34 | size | InsufficientKeySize.go:18:7:18:10 | 1024 : int | InsufficientKeySize.go:26:31:26:34 | size | The size of RSA key '$@' should be at least 2048 bits. | InsufficientKeySize.go:26:31:26:34 | size | 1024 | diff --git a/ql/test/experimental/CWE-326/InsufficientKeySize.go b/ql/test/experimental/CWE-326/InsufficientKeySize.go new file mode 100644 index 00000000000..f0ec53dd628 --- /dev/null +++ b/ql/test/experimental/CWE-326/InsufficientKeySize.go @@ -0,0 +1,34 @@ +package main + +import ( + "crypto/rand" + "crypto/rsa" +) + +func foo1() { + rsa.GenerateKey(rand.Reader, 1024) // BAD +} + +func foo2() { + size := 1024 + rsa.GenerateKey(rand.Reader, size) // BAD +} + +func foo3() { + foo5(1024) // BAD +} + +func foo4() { + foo5(2048) // GOOD +} + +func foo5(size int) { + rsa.GenerateKey(rand.Reader, size) +} + +func main() { + foo1() + foo2() + foo3() + foo4() +} \ No newline at end of file diff --git a/ql/test/experimental/CWE-326/InsufficientKeySize.qlref b/ql/test/experimental/CWE-326/InsufficientKeySize.qlref new file mode 100644 index 00000000000..0eb444e257a --- /dev/null +++ b/ql/test/experimental/CWE-326/InsufficientKeySize.qlref @@ -0,0 +1 @@ +experimental/CWE-326/InsufficientKeySize.ql From 8414759f7dc647dad679ba21b99b415225a5202a Mon Sep 17 00:00:00 2001 From: edvraa <80588099+edvraa@users.noreply.github.com> Date: Tue, 4 May 2021 19:03:58 +0300 Subject: [PATCH 2/4] Code review --- ql/src/experimental/CWE-326/InsufficientKeySize.ql | 12 ++++++------ .../experimental/CWE-326/InsufficientKeySizeBad.go | 2 +- .../experimental/CWE-326/InsufficientKeySizeGood.go | 2 +- ql/test/experimental/CWE-326/InsufficientKeySize.go | 7 ------- 4 files changed, 8 insertions(+), 15 deletions(-) diff --git a/ql/src/experimental/CWE-326/InsufficientKeySize.ql b/ql/src/experimental/CWE-326/InsufficientKeySize.ql index c42def27a4c..9f86ad02927 100644 --- a/ql/src/experimental/CWE-326/InsufficientKeySize.ql +++ b/ql/src/experimental/CWE-326/InsufficientKeySize.ql @@ -11,19 +11,19 @@ import go import DataFlow::PathGraph +/** + * RSA key length data flow tracking configuration. + */ class RsaKeyTrackingConfiguration extends DataFlow::Configuration { RsaKeyTrackingConfiguration() { this = "RsaKeyTrackingConfiguration" } override predicate isSource(DataFlow::Node source) { - exists(ValueExpr c | - source.asExpr() = c and - c.getIntValue() < 2048 - ) + source.asExpr().(ValueExpr).getIntValue() < 2048 } override predicate isSink(DataFlow::Node sink) { - exists(CallExpr c | - sink.asExpr() = c.getArgument(1) and + exists(DataFlow::CallNode c | + sink = c.getArgument(1) and c.getTarget().hasQualifiedName("crypto/rsa", "GenerateKey") ) } diff --git a/ql/src/experimental/CWE-326/InsufficientKeySizeBad.go b/ql/src/experimental/CWE-326/InsufficientKeySizeBad.go index 73abf362758..22682bcdef3 100644 --- a/ql/src/experimental/CWE-326/InsufficientKeySizeBad.go +++ b/ql/src/experimental/CWE-326/InsufficientKeySizeBad.go @@ -13,4 +13,4 @@ func main() { fmt.Println(err) } fmt.Println(pvk) -} \ No newline at end of file +} diff --git a/ql/src/experimental/CWE-326/InsufficientKeySizeGood.go b/ql/src/experimental/CWE-326/InsufficientKeySizeGood.go index f72efecd5c6..b64053c57d5 100644 --- a/ql/src/experimental/CWE-326/InsufficientKeySizeGood.go +++ b/ql/src/experimental/CWE-326/InsufficientKeySizeGood.go @@ -13,4 +13,4 @@ func main() { fmt.Println(err) } fmt.Println(pvk) -} \ No newline at end of file +} diff --git a/ql/test/experimental/CWE-326/InsufficientKeySize.go b/ql/test/experimental/CWE-326/InsufficientKeySize.go index f0ec53dd628..ad1f1c8de75 100644 --- a/ql/test/experimental/CWE-326/InsufficientKeySize.go +++ b/ql/test/experimental/CWE-326/InsufficientKeySize.go @@ -25,10 +25,3 @@ func foo4() { func foo5(size int) { rsa.GenerateKey(rand.Reader, size) } - -func main() { - foo1() - foo2() - foo3() - foo4() -} \ No newline at end of file From c9c22fd87176ab1375ca4810346e15145cfdf88a Mon Sep 17 00:00:00 2001 From: edvraa <80588099+edvraa@users.noreply.github.com> Date: Tue, 11 May 2021 12:59:04 +0300 Subject: [PATCH 3/4] Change the message --- ql/src/experimental/CWE-326/InsufficientKeySize.ql | 3 +-- ql/test/experimental/CWE-326/InsufficientKeySize.expected | 6 +++--- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/ql/src/experimental/CWE-326/InsufficientKeySize.ql b/ql/src/experimental/CWE-326/InsufficientKeySize.ql index 9f86ad02927..f5a4bc969a2 100644 --- a/ql/src/experimental/CWE-326/InsufficientKeySize.ql +++ b/ql/src/experimental/CWE-326/InsufficientKeySize.ql @@ -31,5 +31,4 @@ class RsaKeyTrackingConfiguration extends DataFlow::Configuration { from RsaKeyTrackingConfiguration cfg, DataFlow::PathNode source, DataFlow::PathNode sink where cfg.hasFlowPath(source, sink) -select sink, source, sink, "The size of RSA key '$@' should be at least 2048 bits.", sink, - source.getNode().toString() +select sink, source, sink, "The size of this RSA key should be at least 2048 bits." diff --git a/ql/test/experimental/CWE-326/InsufficientKeySize.expected b/ql/test/experimental/CWE-326/InsufficientKeySize.expected index d02a211a172..c7840f6adb5 100644 --- a/ql/test/experimental/CWE-326/InsufficientKeySize.expected +++ b/ql/test/experimental/CWE-326/InsufficientKeySize.expected @@ -10,6 +10,6 @@ nodes | InsufficientKeySize.go:25:11:25:14 | definition of size : int | semmle.label | definition of size : int | | InsufficientKeySize.go:26:31:26:34 | size | semmle.label | size | #select -| InsufficientKeySize.go:9:31:9:34 | 1024 | InsufficientKeySize.go:9:31:9:34 | 1024 | InsufficientKeySize.go:9:31:9:34 | 1024 | The size of RSA key '$@' should be at least 2048 bits. | InsufficientKeySize.go:9:31:9:34 | 1024 | 1024 | -| InsufficientKeySize.go:14:31:14:34 | size | InsufficientKeySize.go:13:10:13:13 | 1024 : int | InsufficientKeySize.go:14:31:14:34 | size | The size of RSA key '$@' should be at least 2048 bits. | InsufficientKeySize.go:14:31:14:34 | size | 1024 | -| InsufficientKeySize.go:26:31:26:34 | size | InsufficientKeySize.go:18:7:18:10 | 1024 : int | InsufficientKeySize.go:26:31:26:34 | size | The size of RSA key '$@' should be at least 2048 bits. | InsufficientKeySize.go:26:31:26:34 | size | 1024 | +| InsufficientKeySize.go:9:31:9:34 | 1024 | InsufficientKeySize.go:9:31:9:34 | 1024 | InsufficientKeySize.go:9:31:9:34 | 1024 | The size of this RSA key should be at least 2048 bits. | +| InsufficientKeySize.go:14:31:14:34 | size | InsufficientKeySize.go:13:10:13:13 | 1024 : int | InsufficientKeySize.go:14:31:14:34 | size | The size of this RSA key should be at least 2048 bits. | +| InsufficientKeySize.go:26:31:26:34 | size | InsufficientKeySize.go:18:7:18:10 | 1024 : int | InsufficientKeySize.go:26:31:26:34 | size | The size of this RSA key should be at least 2048 bits. | From c95295aa8129322d8930642895010ef51627698c Mon Sep 17 00:00:00 2001 From: edvraa <80588099+edvraa@users.noreply.github.com> Date: Tue, 11 May 2021 13:01:26 +0300 Subject: [PATCH 4/4] Simplify get int --- ql/src/experimental/CWE-326/InsufficientKeySize.ql | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/ql/src/experimental/CWE-326/InsufficientKeySize.ql b/ql/src/experimental/CWE-326/InsufficientKeySize.ql index f5a4bc969a2..ab5637f2914 100644 --- a/ql/src/experimental/CWE-326/InsufficientKeySize.ql +++ b/ql/src/experimental/CWE-326/InsufficientKeySize.ql @@ -17,9 +17,7 @@ import DataFlow::PathGraph class RsaKeyTrackingConfiguration extends DataFlow::Configuration { RsaKeyTrackingConfiguration() { this = "RsaKeyTrackingConfiguration" } - override predicate isSource(DataFlow::Node source) { - source.asExpr().(ValueExpr).getIntValue() < 2048 - } + override predicate isSource(DataFlow::Node source) { source.getIntValue() < 2048 } override predicate isSink(DataFlow::Node sink) { exists(DataFlow::CallNode c |