Skip to content

Rust: Update StreamCipherInit to use getCanonicalPath. #20238

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Aug 19, 2025
Merged

Rust: Update StreamCipherInit to use getCanonicalPath. #20238

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
aafdf1a
Rust: Update StreamCipherInit to use getCanonicalPath.
geoffw0 Jul 25, 2025
a9650e0
Rust: Add a slightly simpler / more explicit test case.
geoffw0 Aug 18, 2025
402e901
Merge branch 'main' into scinit
geoffw0 Aug 18, 2025
fdec780
Rust: Accept consistency .expected changes.
geoffw0 Aug 18, 2025
bf33d1b
Rust: Make a couple of new imports private.
geoffw0 Aug 18, 2025
ab49c33
Merge branch 'main' into scinit
geoffw0 Aug 19, 2025
9f04de8
Rust: Update test results following merge.
geoffw0 Aug 19, 2025
401315c
Update rust/ql/lib/codeql/rust/frameworks/rustcrypto/RustCrypto.qll
geoffw0 Aug 19, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 15 additions & 20 deletions rust/ql/lib/codeql/rust/frameworks/rustcrypto/RustCrypto.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -21,28 +23,21 @@

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 |

Check warning

Code scanning / CodeQL

Omittable 'exists' variable Warning

This exists variable can be omitted by using a don't-care expression
in this argument
Loading
.
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)
)
}

Expand Down
23 changes: 6 additions & 17 deletions rust/ql/test/query-tests/security/CWE-327/BrokenCryptoAlgorithm.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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: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 |
| test_cipher.rs:71:23:71:46 | ...::new_from_slice(...) | $@ is broken or weak, and should not be used. | test_cipher.rs:71:23:71:46 | ...::new_from_slice(...) | The cryptographic algorithm DES |
| test_cipher.rs:96:24:96:52 | ...::new_from_slice(...) | $@ is broken or weak, and should not be used. | test_cipher.rs:96:24:96:52 | ...::new_from_slice(...) | The cryptographic algorithm 3DES |
| test_cipher.rs:96:24:96:52 | ...::new_from_slice(...) | $@ is broken or weak, and should not be used. | test_cipher.rs:96:24:96:52 | ...::new_from_slice(...) | The cryptographic algorithm DES |
| test_cipher.rs:109:23:109:56 | ...::new_with_eff_key_len(...) | $@ is broken or weak, and should not be used. | test_cipher.rs:109:23:109:56 | ...::new_with_eff_key_len(...) | The cryptographic algorithm RC2 |
| test_cipher.rs:114:23:114:50 | ...::new(...) | $@ is broken or weak, and should not be used. | test_cipher.rs:114:23:114:50 | ...::new(...) | The cryptographic algorithm RC5 |
| test_cipher.rs:118:23:118:55 | ...::new_from_slice(...) | $@ is broken or weak, and should not be used. | test_cipher.rs:118:23:118:55 | ...::new_from_slice(...) | The cryptographic algorithm RC5 |
30 changes: 17 additions & 13 deletions rust/ql/test/query-tests/security/CWE-327/test_cipher.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ fn test_stream_cipher(

fn test_block_cipher(
key: &[u8], key128: &[u8;16], key192: &[u8;24], key256: &[u8;32],
data: &mut [u8], input: &[u8], block128: &mut [u8;16]
data: &mut [u8], input: &[u8], block128: &mut [u8;16], des_key : &cipher::Key<Des>
) {
// aes
let aes_cipher1 = Aes128::new(key128.into());
Expand All @@ -56,36 +56,40 @@ 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_cipher0 : Des = Des::new(des_key); // $ MISSING: Alert[rust/weak-cryptographic-algorithm]
des_cipher0.encrypt_block(data.into());
des_cipher0.decrypt_block(data.into());

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());

Expand All @@ -94,11 +98,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());

Expand Down Expand Up @@ -129,15 +133,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]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I note that this call and the two below (also) do not resolve because we currently do not support blanket implementations.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, and I note the issue tracking this. 👍

_ = 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();
}
Loading