Merge pull request #594 from owen-mc/insufficient-key-size-barrier-guard

Add barrier guard for comparison in Insufficient Key Size query

Author: smowton

change-notes/2021-10-27-insufficient-key-size-sanitizer.md

@@ -0,0 +1,3 @@
+lgtm,codescanning
+* The query "Use of a weak cryptographic key" has been improved to recognize more cases where the
+  key size should be considered to be safe, which should lead to fewer false positive results.

ql/src/Security/CWE-326/InsufficientKeySize.ql

@@ -27,6 +27,25 @@ class RsaKeyTrackingConfiguration extends DataFlow::Configuration {
       c.getTarget().hasQualifiedName("crypto/rsa", "GenerateKey")
     )
   }
+
+  override predicate isBarrierGuard(DataFlow::BarrierGuard guard) {
+    guard instanceof ComparisonBarrierGuard
+  }
+}
+
+/**
+ * A comparison which guarantees that an expression is at least 2048,
+ * considered as a barrier guard for key sizes.
+ */
+class ComparisonBarrierGuard extends DataFlow::BarrierGuard instanceof DataFlow::RelationalComparisonNode {
+  override predicate checks(Expr e, boolean branch) {
+    exists(DataFlow::Node lesser, DataFlow::Node greater, int bias |
+      super.leq(branch, lesser, greater, bias)
+    |
+      globalValueNumber(DataFlow::exprNode(e)) = globalValueNumber(greater) and
+      lesser.getIntValue() - bias >= 2048
+    )
+  }
 }
 
 from RsaKeyTrackingConfiguration cfg, DataFlow::PathNode source, DataFlow::PathNode sink

ql/test/query-tests/Security/CWE-326/InsufficientKeySize.expected

@@ -2,14 +2,26 @@ 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 |
+| InsufficientKeySize.go:30:13:30:16 | 1024 : int | InsufficientKeySize.go:32:32:32:38 | keyBits |
+| InsufficientKeySize.go:44:13:44:16 | 1024 : int | InsufficientKeySize.go:47:32:47:38 | keyBits |
+| InsufficientKeySize.go:61:21:61:24 | 1024 : int | InsufficientKeySize.go:67:31:67:37 | keyBits |
 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 |
+| InsufficientKeySize.go:30:13:30:16 | 1024 : int | semmle.label | 1024 : int |
+| InsufficientKeySize.go:32:32:32:38 | keyBits | semmle.label | keyBits |
+| InsufficientKeySize.go:44:13:44:16 | 1024 : int | semmle.label | 1024 : int |
+| InsufficientKeySize.go:47:32:47:38 | keyBits | semmle.label | keyBits |
+| InsufficientKeySize.go:61:21:61:24 | 1024 : int | semmle.label | 1024 : int |
+| InsufficientKeySize.go:67:31:67:37 | keyBits | semmle.label | keyBits |
 #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 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. |
+| InsufficientKeySize.go:32:32:32:38 | keyBits | InsufficientKeySize.go:30:13:30:16 | 1024 : int | InsufficientKeySize.go:32:32:32:38 | keyBits | The size of this RSA key should be at least 2048 bits. |
+| InsufficientKeySize.go:47:32:47:38 | keyBits | InsufficientKeySize.go:44:13:44:16 | 1024 : int | InsufficientKeySize.go:47:32:47:38 | keyBits | The size of this RSA key should be at least 2048 bits. |
+| InsufficientKeySize.go:67:31:67:37 | keyBits | InsufficientKeySize.go:61:21:61:24 | 1024 : int | InsufficientKeySize.go:67:31:67:37 | keyBits | The size of this RSA key should be at least 2048 bits. |

ql/test/query-tests/Security/CWE-326/InsufficientKeySize.go

@@ -25,3 +25,57 @@ func foo4() {
 func foo5(size int) {
 	rsa.GenerateKey(rand.Reader, size)
 }
+
+func foo6() {
+	keyBits := 1024
+	if keyBits >= 2047 {
+		rsa.GenerateKey(rand.Reader, keyBits) // BAD
+	}
+}
+
+func foo7() {
+	keyBits := 1024
+	if keyBits >= 2048 {
+		rsa.GenerateKey(rand.Reader, keyBits) // GOOD
+	}
+}
+
+func foo8() {
+	keyBits := 1024
+	switch {
+	case keyBits >= 2047:
+		rsa.GenerateKey(rand.Reader, keyBits) // BAD
+	}
+}
+
+func foo9() {
+	keyBits := 1024
+	switch {
+	case keyBits >= 2048:
+		rsa.GenerateKey(rand.Reader, keyBits) // GOOD
+	}
+}
+
+func foo10(customOptionSupplied bool, nonConstantKeyBits int) {
+	keyBits := 0
+	constantKeyBits := 1024
+	if customOptionSupplied {
+		keyBits = constantKeyBits
+	} else {
+		keyBits = nonConstantKeyBits
+	}
+	rsa.GenerateKey(rand.Reader, keyBits) // BAD
+}
+
+func foo11(customOptionSupplied bool, nonConstantKeyBits int) {
+	keyBits := 0
+	constantKeyBits := 1024
+	if customOptionSupplied {
+		keyBits = constantKeyBits
+	} else {
+		keyBits = nonConstantKeyBits
+	}
+	if keyBits >= 2048 {
+		rsa.GenerateKey(rand.Reader, keyBits) // GOOD
+	}
+}