Rust: Update StreamCipherInit to use getCanonicalPath.

Author: geoffw0

rust/ql/lib/codeql/rust/frameworks/rustcrypto/RustCrypto.qll

@@ -5,6 +5,8 @@
 private import rust
 private import codeql.rust.Concepts
 private import codeql.rust.dataflow.DataFlow
+import codeql.rust.internal.TypeInference
+import codeql.rust.internal.Type
 
 bindingset[algorithmName]
 private string simplifyAlgorithmName(string algorithmName) {
@@ -21,28 +23,21 @@ class StreamCipherInit extends Cryptography::CryptographicOperation::Range {
 
   StreamCipherInit() {
     // a call to `cipher::KeyInit::new`, `cipher::KeyInit::new_from_slice`,
-    // `cipher::KeyIvInit::new`, `cipher::KeyIvInit::new_from_slices` or `rc2::Rc2::new_with_eff_key_len`.
-    exists(PathExpr p, string rawAlgorithmName |
-      this.asExpr().getExpr().(CallExpr).getFunction() = p and
-      p.getResolvedCrateOrigin().matches("%/RustCrypto%") and
-      p.getPath().getText() = ["new", "new_from_slice", "new_from_slices", "new_with_eff_key_len"] and
-      (
-        rawAlgorithmName = p.getPath().getQualifier().getText() or
+    // `cipher::KeyIvInit::new`, `cipher::KeyIvInit::new_from_slices`, `rc2::Rc2::new_with_eff_key_len` or similar.
+    exists(CallExprBase ce, string rawAlgorithmName |
+      ce = this.asExpr().getExpr() and
+      ce.getStaticTarget()
+          .getCanonicalPath()
+          .matches(["%::new", "%::new_from_slice", "%::new_with_eff_key_len", "%::new_from_slices"]) and
+      // extract the algorithm name from the type of `ce` or its receiver.
+      exists(Type t, TypePath tp |
+        t = inferType([ce, ce.(MethodCallExpr).getReceiver()], tp) and
         rawAlgorithmName =
-          p.getPath()
-              .getQualifier()
-              .getSegment()
-              .getGenericArgList()
-              .getGenericArg(0)
-              .(TypeArg)
-              .getTypeRepr()
-              .(PathTypeRepr)
-              .getPath()
-              .getSegment()
-              .getIdentifier()
-              .getText()
+          t.(StructType).asItemNode().(Addressable).getCanonicalPath().splitAt("::")
       ) and
-      algorithmName = simplifyAlgorithmName(rawAlgorithmName)
+      algorithmName = simplifyAlgorithmName(rawAlgorithmName) and
+      // only match a known cryptographic algorithm
+      any(Cryptography::CryptographicAlgorithm alg).matchesName(algorithmName)
     )
   }
 

rust/ql/test/query-tests/security/CWE-327/BrokenCryptoAlgorithm.expected

@@ -2,20 +2,9 @@
 | test_cipher.rs:23:27:23:60 | ...::new_from_slice(...) | $@ is broken or weak, and should not be used. | test_cipher.rs:23:27:23:60 | ...::new_from_slice(...) | The cryptographic algorithm RC4 |
 | test_cipher.rs:26:27:26:48 | ...::new(...) | $@ is broken or weak, and should not be used. | test_cipher.rs:26:27:26:48 | ...::new(...) | The cryptographic algorithm RC4 |
 | test_cipher.rs:29:27:29:48 | ...::new(...) | $@ is broken or weak, and should not be used. | test_cipher.rs:29:27:29:48 | ...::new(...) | The cryptographic algorithm RC4 |
-| test_cipher.rs:59:23:59:42 | ...::new(...) | $@ is broken or weak, and should not be used. | test_cipher.rs:59:23:59:42 | ...::new(...) | The cryptographic algorithm DES |
-| test_cipher.rs:63:23:63:47 | ...::new(...) | $@ is broken or weak, and should not be used. | test_cipher.rs:63:23:63:47 | ...::new(...) | The cryptographic algorithm DES |
 | test_cipher.rs:67:23:67:46 | ...::new_from_slice(...) | $@ is broken or weak, and should not be used. | test_cipher.rs:67:23:67:46 | ...::new_from_slice(...) | The cryptographic algorithm DES |
-| test_cipher.rs:71:23:71:42 | ...::new(...) | $@ is broken or weak, and should not be used. | test_cipher.rs:71:23:71:42 | ...::new(...) | The cryptographic algorithm DES |
-| test_cipher.rs:75:27:75:46 | ...::new(...) | $@ is broken or weak, and should not be used. | test_cipher.rs:75:27:75:46 | ...::new(...) | The cryptographic algorithm DES |
-| test_cipher.rs:80:24:80:48 | ...::new(...) | $@ is broken or weak, and should not be used. | test_cipher.rs:80:24:80:48 | ...::new(...) | The cryptographic algorithm 3DES |
-| test_cipher.rs:84:24:84:48 | ...::new(...) | $@ is broken or weak, and should not be used. | test_cipher.rs:84:24:84:48 | ...::new(...) | The cryptographic algorithm 3DES |
-| test_cipher.rs:88:24:88:48 | ...::new(...) | $@ is broken or weak, and should not be used. | test_cipher.rs:88:24:88:48 | ...::new(...) | The cryptographic algorithm 3DES |
 | test_cipher.rs:92:24:92:52 | ...::new_from_slice(...) | $@ is broken or weak, and should not be used. | test_cipher.rs:92:24:92:52 | ...::new_from_slice(...) | The cryptographic algorithm 3DES |
-| test_cipher.rs:97:23:97:42 | ...::new(...) | $@ is broken or weak, and should not be used. | test_cipher.rs:97:23:97:42 | ...::new(...) | The cryptographic algorithm RC2 |
-| test_cipher.rs:101:23:101:46 | ...::new_from_slice(...) | $@ is broken or weak, and should not be used. | test_cipher.rs:101:23:101:46 | ...::new_from_slice(...) | The cryptographic algorithm RC2 |
+| test_cipher.rs:92:24:92:52 | ...::new_from_slice(...) | $@ is broken or weak, and should not be used. | test_cipher.rs:92:24:92:52 | ...::new_from_slice(...) | The cryptographic algorithm DES |
 | test_cipher.rs:105:23:105:56 | ...::new_with_eff_key_len(...) | $@ is broken or weak, and should not be used. | test_cipher.rs:105:23:105:56 | ...::new_with_eff_key_len(...) | The cryptographic algorithm RC2 |
 | test_cipher.rs:110:23:110:50 | ...::new(...) | $@ is broken or weak, and should not be used. | test_cipher.rs:110:23:110:50 | ...::new(...) | The cryptographic algorithm RC5 |
 | test_cipher.rs:114:23:114:55 | ...::new_from_slice(...) | $@ is broken or weak, and should not be used. | test_cipher.rs:114:23:114:55 | ...::new_from_slice(...) | The cryptographic algorithm RC5 |
-| test_cipher.rs:132:23:132:76 | ...::new(...) | $@ is broken or weak, and should not be used. | test_cipher.rs:132:23:132:76 | ...::new(...) | The cryptographic algorithm DES |
-| test_cipher.rs:138:23:138:76 | ...::new_from_slices(...) | $@ is broken or weak, and should not be used. | test_cipher.rs:138:23:138:76 | ...::new_from_slices(...) | The cryptographic algorithm DES |
-| test_cipher.rs:141:23:141:76 | ...::new(...) | $@ is broken or weak, and should not be used. | test_cipher.rs:141:23:141:76 | ...::new(...) | The cryptographic algorithm DES |

rust/ql/test/query-tests/security/CWE-327/test_cipher.rs

@@ -56,36 +56,36 @@ fn test_block_cipher(
     aes_cipher3.decrypt_block(block128.into());
 
     // des (broken)
-    let des_cipher1 = Des::new(key.into()); // $ Alert[rust/weak-cryptographic-algorithm]
+    let des_cipher1 = Des::new(key.into()); // $ MISSING: Alert[rust/weak-cryptographic-algorithm]
     des_cipher1.encrypt_block(data.into());
     des_cipher1.decrypt_block(data.into());
 
-    let des_cipher2 = des::Des::new(key.into()); // $ Alert[rust/weak-cryptographic-algorithm]
+    let des_cipher2 = des::Des::new(key.into()); // $ MISSING: Alert[rust/weak-cryptographic-algorithm]
     des_cipher2.encrypt_block(data.into());
     des_cipher2.decrypt_block(data.into());
 
     let des_cipher3 = Des::new_from_slice(key).expect("fail"); // $ Alert[rust/weak-cryptographic-algorithm]
     des_cipher3.encrypt_block(data.into());
     des_cipher3.decrypt_block(data.into());
 
-    let des_cipher4 = Des::new(key.into()); // $ Alert[rust/weak-cryptographic-algorithm]
+    let des_cipher4 = Des::new(key.into()); // $ MISSING: Alert[rust/weak-cryptographic-algorithm]
     des_cipher4.encrypt_block_b2b(input.into(), data.into());
     des_cipher4.decrypt_block_b2b(input.into(), data.into());
 
-    let mut des_cipher5 = Des::new(key.into()); // $ Alert[rust/weak-cryptographic-algorithm]
+    let mut des_cipher5 = Des::new(key.into()); // $ MISSING: Alert[rust/weak-cryptographic-algorithm]
     des_cipher5.encrypt_block_mut(data.into());
     des_cipher5.decrypt_block_mut(data.into());
 
     // triple des (broken)
-    let tdes_cipher1 = TdesEde2::new(key.into()); // $ Alert[rust/weak-cryptographic-algorithm]
+    let tdes_cipher1 = TdesEde2::new(key.into()); // $ MISSING: Alert[rust/weak-cryptographic-algorithm]
     tdes_cipher1.encrypt_block(data.into());
     tdes_cipher1.decrypt_block(data.into());
 
-    let tdes_cipher2 = TdesEde3::new(key.into()); // $ Alert[rust/weak-cryptographic-algorithm]
+    let tdes_cipher2 = TdesEde3::new(key.into()); // $ MISSING: Alert[rust/weak-cryptographic-algorithm]
     tdes_cipher2.encrypt_block(data.into());
     tdes_cipher2.decrypt_block(data.into());
 
-    let tdes_cipher3 = TdesEee2::new(key.into()); // $ Alert[rust/weak-cryptographic-algorithm]
+    let tdes_cipher3 = TdesEee2::new(key.into()); // $ MISSING: Alert[rust/weak-cryptographic-algorithm]
     tdes_cipher3.encrypt_block(data.into());
     tdes_cipher3.decrypt_block(data.into());
 
@@ -94,11 +94,11 @@ fn test_block_cipher(
     tdes_cipher4.decrypt_block(data.into());
 
     // rc2 (broken)
-    let rc2_cipher1 = Rc2::new(key.into()); // $ Alert[rust/weak-cryptographic-algorithm]
+    let rc2_cipher1 = Rc2::new(key.into()); // $ MISSING: Alert[rust/weak-cryptographic-algorithm]
     rc2_cipher1.encrypt_block(data.into());
     rc2_cipher1.decrypt_block(data.into());
 
-    let rc2_cipher2 = Rc2::new_from_slice(key).expect("fail"); // $ Alert[rust/weak-cryptographic-algorithm]
+    let rc2_cipher2 = Rc2::new_from_slice(key).expect("fail"); // $ MISSING: Alert[rust/weak-cryptographic-algorithm]
     rc2_cipher2.encrypt_block(data.into());
     rc2_cipher2.decrypt_block(data.into());
 
@@ -129,15 +129,15 @@ fn test_cbc(
     _ = aes_cipher1.encrypt_padded_mut::<aes::cipher::block_padding::Pkcs7>(data, data_len).unwrap();
 
     // des (broken)
-    let des_cipher1 = cbc::Encryptor::<des::Des>::new(key.into(), iv.into()); // $ Alert[rust/weak-cryptographic-algorithm]
+    let des_cipher1 = cbc::Encryptor::<des::Des>::new(key.into(), iv.into()); // $ MISSING: Alert[rust/weak-cryptographic-algorithm]
     _ = des_cipher1.encrypt_padded_mut::<des::cipher::block_padding::Pkcs7>(data, data_len).unwrap();
 
     let des_cipher2 = MyDesEncryptor::new(key.into(), iv.into()); // $ MISSING: Alert[rust/weak-cryptographic-algorithm]
     _ = des_cipher2.encrypt_padded_mut::<des::cipher::block_padding::Pkcs7>(data, data_len).unwrap();
 
-    let des_cipher3 = cbc::Encryptor::<des::Des>::new_from_slices(&key, &iv).unwrap(); // $ Alert[rust/weak-cryptographic-algorithm]
+    let des_cipher3 = cbc::Encryptor::<des::Des>::new_from_slices(&key, &iv).unwrap(); // $ MISSING: Alert[rust/weak-cryptographic-algorithm]
     _ = des_cipher3.encrypt_padded_mut::<des::cipher::block_padding::Pkcs7>(data, data_len).unwrap();
 
-    let des_cipher4 = cbc::Encryptor::<des::Des>::new(key.into(), iv.into()); // $ Alert[rust/weak-cryptographic-algorithm]
+    let des_cipher4 = cbc::Encryptor::<des::Des>::new(key.into(), iv.into()); // $ MISSING: Alert[rust/weak-cryptographic-algorithm]
     _ = des_cipher4.encrypt_padded_b2b_mut::<des::cipher::block_padding::Pkcs7>(input, data).unwrap();
 }