Rust: Improve the model.

Author: geoffw0

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

@@ -6,6 +6,12 @@ private import rust
 private import codeql.rust.Concepts
 private import codeql.rust.dataflow.DataFlow
 
+bindingset[algorithmName]
+string simplifyAlgorithmName(string algorithmName) {
+  // the cipher library gives triple-DES names like "TdesEee2" and "TdesEde2"
+  if algorithmName.matches("Tdes%") then result = "3des" else result = algorithmName
+}
+
 /**
  * An operation that initializes a cipher through the `cipher::KeyInit` or
  * `cipher::KeyIvInit` trait, for example `Des::new` or `cbc::Encryptor<des::Des>::new`.
@@ -15,13 +21,17 @@ class StreamCipherInit extends Cryptography::CryptographicOperation::Range, Data
 
   StreamCipherInit() {
     // a call to `cipher::KeyInit::new`, `cipher::KeyInit::new_from_slice`,
-    // `cipher::KeyIvInit::new` or `cipher::KeyIvInit::new_from_slices`.
-    exists(Path p |
+    // `cipher::KeyIvInit::new`, `cipher::KeyIvInit::new_from_slices` or `rc2::Rc2::new_with_eff_key_len`.
+    exists(Path p, string rawAlgorithmName |
       this.asExpr().getExpr().(CallExpr).getFunction().(PathExpr).getPath() = p and
       p.getResolvedCrateOrigin().matches("%/RustCrypto%") and
       p.getPart().getNameRef().getText() =
-        ["new", "new_from_slice", "new_from_slices"] and
-      algorithmName = p.getQualifier().getPart().getNameRef().getText()
+        ["new", "new_from_slice", "new_from_slices", "new_with_eff_key_len"] and
+      (
+        rawAlgorithmName = p.getQualifier().getPart().getNameRef().getText() or
+        rawAlgorithmName = p.getQualifier().getPart().getGenericArgList().getGenericArg(0).(TypeArg).getTy().(PathType).getPath().getPart().getNameRef().getText()
+      ) and
+      algorithmName = simplifyAlgorithmName(rawAlgorithmName)
     )
   }
 

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

@@ -7,7 +7,15 @@
 | 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: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

@@ -77,19 +77,19 @@ fn test_block_cipher(
     des_cipher5.decrypt_block_mut(data.into());
 
     // triple des (broken)
-    let tdes_cipher1 = TdesEde2::new(key.into()); // $ MISSING: Alert[rust/weak-cryptographic-algorithm]
+    let tdes_cipher1 = TdesEde2::new(key.into()); // $ Alert[rust/weak-cryptographic-algorithm]
     tdes_cipher1.encrypt_block(data.into());
     tdes_cipher1.decrypt_block(data.into());
 
-    let tdes_cipher2 = TdesEde3::new(key.into()); // $ MISSING: Alert[rust/weak-cryptographic-algorithm]
+    let tdes_cipher2 = TdesEde3::new(key.into()); // $ Alert[rust/weak-cryptographic-algorithm]
     tdes_cipher2.encrypt_block(data.into());
     tdes_cipher2.decrypt_block(data.into());
 
-    let tdes_cipher3 = TdesEee2::new(key.into()); // $ MISSING: Alert[rust/weak-cryptographic-algorithm]
+    let tdes_cipher3 = TdesEee2::new(key.into()); // $ Alert[rust/weak-cryptographic-algorithm]
     tdes_cipher3.encrypt_block(data.into());
     tdes_cipher3.decrypt_block(data.into());
 
-    let tdes_cipher4 = TdesEee3::new_from_slice(key).unwrap(); // $ MISSING: Alert[rust/weak-cryptographic-algorithm]
+    let tdes_cipher4 = TdesEee3::new_from_slice(key).unwrap(); // $ Alert[rust/weak-cryptographic-algorithm]
     tdes_cipher4.encrypt_block(data.into());
     tdes_cipher4.decrypt_block(data.into());
 
@@ -102,7 +102,7 @@ fn test_block_cipher(
     rc2_cipher2.encrypt_block(data.into());
     rc2_cipher2.decrypt_block(data.into());
 
-    let rc2_cipher3 = Rc2::new_with_eff_key_len(key, 64); // $ MISSING: Alert[rust/weak-cryptographic-algorithm]
+    let rc2_cipher3 = Rc2::new_with_eff_key_len(key, 64); // $ Alert[rust/weak-cryptographic-algorithm]
     rc2_cipher3.encrypt_block(data.into());
     rc2_cipher3.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()); // $ MISSING: Alert[rust/weak-cryptographic-algorithm]
+    let des_cipher1 = cbc::Encryptor::<des::Des>::new(key.into(), iv.into()); // $ 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(); // $ MISSING: Alert[rust/weak-cryptographic-algorithm]
+    let des_cipher3 = cbc::Encryptor::<des::Des>::new_from_slices(&key, &iv).unwrap(); // $ 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()); // $ MISSING: Alert[rust/weak-cryptographic-algorithm]
+    let des_cipher4 = cbc::Encryptor::<des::Des>::new(key.into(), iv.into()); // $ Alert[rust/weak-cryptographic-algorithm]
     _ = des_cipher4.encrypt_padded_b2b_mut::<des::cipher::block_padding::Pkcs7>(input, data).unwrap();
 }