Bug fix in InvalidKeySize.ql

Author: fegge

cpp/src/crypto/InvalidKeySize.ql

@@ -1,7 +1,7 @@
 /**
  * @name Invalid key size
  * @id tob/cpp/invalid-key-size
- * @description Tests if keys passed to EncryptInit_ex have the same size as the key size of the cipher used
+ * @description Tests if keys passed to EVP_EncryptInit and EVP_EncryptInit_ex have the same size as the key size of the cipher used
  * @kind problem
  * @tags correctness crypto
  * @problem.severity warning
@@ -41,9 +41,21 @@ class Key extends Variable {
     cipher.getKeySize() = this.getSize()
   }
 
-
+  // Avoid matching on pointers where the key size is not known.
+  predicate isArray() {
+    this.getType() instanceof ArrayType
+  }
 }
 
-from Key key, EVP_CIPHER cipher
-where cipher = key.getACipher() and not key.correctKeySize(cipher)
-select key.getInitCall().getLocation(), "Key size (" + key.getSize() + " bytes) does not match the expected key size for the encryption algorithm (" + cipher.getKeySize() + " bytes)"
+
+
+from 
+  Key key, 
+  EVP_CIPHER cipher
+where 
+  cipher = key.getACipher() and 
+  key.isArray() and
+  not key.correctKeySize(cipher)
+select 
+  key.getInitCall().getLocation(),
+  "Key size (" + key.getSize() + " bytes) does not match the expected key size for the encryption algorithm (" + cipher.getKeySize() + " bytes)"

cpp/test/query-tests/crypto/InvalidKeySize/InvalidKeySize.expected

@@ -1 +1 @@
-| test.c:15:5:15:22 | test.c:15:5:15:22 | Key size (31 bytes) does not match the expected key size for the encryption algorithm (32 bytes) |
+| test.c:9:3:9:20 | test.c:9:3:9:20 | Key size (16 bytes) does not match the expected key size for the encryption algorithm (32 bytes) |

cpp/test/query-tests/crypto/InvalidKeySize/test.c

@@ -1,18 +1,31 @@
 #include "../../../include/openssl/evp.h"
 #include "../../../include/openssl/rand.h"
 
-int main(void)
-{
-    unsigned char key[31];  // should be 32 for 256 bit key
+void found(EVP_CIPHER_CTX *ctx) {
+  unsigned char key[16]; // should be 32 for 256 bit key
+  unsigned char *iv = (unsigned char *)"0123456789012345";
 
-    const EVP_CIPHER *c = EVP_aes_128_cbc();
+  RAND_bytes(key, sizeof(key));
+  EVP_EncryptInit_ex(ctx, EVP_aes_256_cbc(), NULL, key, iv);
+}
 
-    int rc = RAND_bytes(key, sizeof(key));
+void notFound(EVP_CIPHER_CTX *ctx, unsigned char *key) {
+  unsigned char *iv = (unsigned char *)"0123456789012345";
 
-    /* A 128 bit IV */
-    unsigned char *iv = (unsigned char *)"0123456789012345";
+  EVP_EncryptInit_ex(ctx, EVP_aes_256_cbc(), NULL, key, iv);
+}
 
-    EVP_EncryptInit_ex(EVP_CIPHER_CTX_new(), EVP_aes_256_cbc(), NULL, key, iv);
+int main(void) {
+  {
+    EVP_CIPHER_CTX *ctx = EVP_CIPHER_CTX_new();
+    found(ctx);
+  }
+  {
+    unsigned char key[16]; // should be 32 for 256 bit key
+    RAND_bytes(key, sizeof(key));
+    EVP_CIPHER_CTX *ctx = EVP_CIPHER_CTX_new();
+    notFound(ctx, key);
+  }
 
-    return 0;
-}
+  return 0;
+}