Skip to content

Commit 2665d83

Browse files
geoffw0geoffw0
authored
Merge pull request #20939 from geoffw0/saltmodel
Rust: Add heuristic sinks for passwords, initialization vectors etc
2 parents ca9d327 + 3028e5d commit 2665d83

File tree

5 files changed

+156
-9
lines changed

5 files changed

+156
-9
lines changed

‎rust/ql/lib/codeql/rust/security/HardcodedCryptographicValueExtensions.qll‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ private import codeql.rust.dataflow.FlowSource
99
private import codeql.rust.dataflow.FlowSink
1010
private import codeql.rust.Concepts
1111
private import codeql.rust.security.SensitiveData
12+
private import codeql.rust.dataflow.internal.Node as Node
1213

1314
/**
1415
* A kind of cryptographic value.
@@ -98,6 +99,37 @@ module HardcodedCryptographicValue {
9899
override CryptographicValueKind getKind() { result = kind }
99100
}
100101

102+
/**
103+
* A heuristic sink for hard-coded cryptographic value vulnerabilities.
104+
*/
105+
private class HeuristicSinks extends Sink {
106+
CryptographicValueKind kind;
107+
108+
HeuristicSinks() {
109+
// any argument going to a parameter whose name matches a credential name
110+
exists(Call c, Function f, int argIndex, string argName |
111+
c.getPositionalArgument(argIndex) = this.asExpr() and
112+
c.getStaticTarget() = f and
113+
f.getParam(argIndex).getPat().(IdentPat).getName().getText() = argName and
114+
(
115+
argName = "password" and kind = "password"
116+
or
117+
argName = "iv" and kind = "iv"
118+
or
119+
argName = "nonce" and kind = "nonce"
120+
or
121+
argName = "salt" and kind = "salt"
122+
//
123+
// note: matching "key" results in too many false positives
124+
) and
125+
// don't duplicate modeled sinks
126+
not exists(ModelsAsDataSinks s | s.(Node::FlowSummaryNode).getSinkElement().getCall() = c)
127+
)
128+
}
129+
130+
override CryptographicValueKind getKind() { result = kind }
131+
}
132+
101133
/**
102134
* A call to `getrandom` that is a barrier.
103135
*/

‎rust/ql/src/change-notes/2025-12-01-hard-coded-cryptographic-value.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* The `rust/hard-coded-cryptographic-value` query has been extended with new heuristic sinks identifying passwords, initialization vectors, nonces and salts.

‎rust/ql/src/queries/security/CWE-798/HardcodedCryptographicValue.ql‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,11 @@ module HardcodedCryptographicValueConfig implements DataFlow::ConfigSig {
3939
// case like `[0, 0, 0, 0]`)
4040
isSource(node)
4141
}
42+
43+
predicate isBarrierOut(DataFlow::Node node) {
44+
// make sinks barriers so that we only report the closest instance
45+
isSink(node)
46+
}
4247
}
4348

4449
module HardcodedCryptographicValueFlow = TaintTracking::Global<HardcodedCryptographicValueConfig>;

‎rust/ql/test/query-tests/security/CWE-798/HardcodedCryptographicValue.expected‎

Lines changed: 44 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,37 +10,45 @@
1010
| test_cookie.rs:21:28:21:34 | [0; 64] | test_cookie.rs:21:28:21:34 | [0; 64] | test_cookie.rs:22:16:22:24 | ...::from | This hard-coded value is used as $@. | test_cookie.rs:22:16:22:24 | ...::from | a key |
1111
| test_cookie.rs:38:28:38:36 | [0u8; 64] | test_cookie.rs:38:28:38:36 | [0u8; 64] | test_cookie.rs:42:14:42:32 | ...::from | This hard-coded value is used as $@. | test_cookie.rs:42:14:42:32 | ...::from | a key |
1212
| test_cookie.rs:49:23:49:25 | 0u8 | test_cookie.rs:49:23:49:25 | 0u8 | test_cookie.rs:53:14:53:32 | ...::from | This hard-coded value is used as $@. | test_cookie.rs:53:14:53:32 | ...::from | a key |
13+
| test_heuristic.rs:44:31:44:38 | [0u8; 16] | test_heuristic.rs:44:31:44:38 | [0u8; 16] | test_heuristic.rs:45:41:45:48 | const_iv | This hard-coded value is used as $@. | test_heuristic.rs:45:41:45:48 | const_iv | an initialization vector |
14+
| test_heuristic.rs:63:30:63:37 | "secret" | test_heuristic.rs:63:30:63:37 | "secret" | test_heuristic.rs:63:30:63:37 | "secret" | This hard-coded value is used as $@. | test_heuristic.rs:63:30:63:37 | "secret" | a password |
15+
| test_heuristic.rs:64:20:64:27 | [0u8; 16] | test_heuristic.rs:64:20:64:27 | [0u8; 16] | test_heuristic.rs:64:19:64:27 | &... | This hard-coded value is used as $@. | test_heuristic.rs:64:19:64:27 | &... | a nonce |
16+
| test_heuristic.rs:65:31:65:38 | [0u8; 16] | test_heuristic.rs:65:31:65:38 | [0u8; 16] | test_heuristic.rs:65:30:65:38 | &... | This hard-coded value is used as $@. | test_heuristic.rs:65:30:65:38 | &... | a salt |
17+
| test_heuristic.rs:67:22:67:22 | 0 | test_heuristic.rs:67:22:67:22 | 0 | test_heuristic.rs:67:22:67:22 | 0 | This hard-coded value is used as $@. | test_heuristic.rs:67:22:67:22 | 0 | a salt |
18+
| test_heuristic.rs:69:32:69:32 | 1 | test_heuristic.rs:69:32:69:32 | 1 | test_heuristic.rs:69:22:69:32 | ... + ... | This hard-coded value is used as $@. | test_heuristic.rs:69:22:69:32 | ... + ... | a salt |
19+
| test_heuristic.rs:70:34:70:35 | 32 | test_heuristic.rs:70:34:70:35 | 32 | test_heuristic.rs:70:22:70:62 | ... ^ ... | This hard-coded value is used as $@. | test_heuristic.rs:70:22:70:62 | ... ^ ... | a salt |
20+
| test_heuristic.rs:70:52:70:61 | 0xFFFFFFFF | test_heuristic.rs:70:52:70:61 | 0xFFFFFFFF | test_heuristic.rs:70:22:70:62 | ... ^ ... | This hard-coded value is used as $@. | test_heuristic.rs:70:22:70:62 | ... ^ ... | a salt |
1321
edges
1422
| test_cipher.rs:18:9:18:14 | const1 [&ref] | test_cipher.rs:19:73:19:78 | const1 [&ref] | provenance | |
1523
| test_cipher.rs:18:28:18:36 | &... [&ref] | test_cipher.rs:18:9:18:14 | const1 [&ref] | provenance | |
1624
| test_cipher.rs:18:29:18:36 | [0u8; 16] | test_cipher.rs:18:28:18:36 | &... [&ref] | provenance | |
1725
| test_cipher.rs:19:49:19:79 | ...::from_slice(...) [&ref] | test_cipher.rs:19:30:19:47 | ...::new | provenance | MaD:3 Sink:MaD:3 |
18-
| test_cipher.rs:19:73:19:78 | const1 [&ref] | test_cipher.rs:19:49:19:79 | ...::from_slice(...) [&ref] | provenance | MaD:9 |
26+
| test_cipher.rs:19:73:19:78 | const1 [&ref] | test_cipher.rs:19:49:19:79 | ...::from_slice(...) [&ref] | provenance | MaD:10 |
1927
| test_cipher.rs:25:9:25:14 | const4 [&ref] | test_cipher.rs:26:66:26:71 | const4 [&ref] | provenance | |
2028
| test_cipher.rs:25:28:25:36 | &... [&ref] | test_cipher.rs:25:9:25:14 | const4 [&ref] | provenance | |
2129
| test_cipher.rs:25:29:25:36 | [0u8; 16] | test_cipher.rs:25:28:25:36 | &... [&ref] | provenance | |
2230
| test_cipher.rs:26:42:26:72 | ...::from_slice(...) [&ref] | test_cipher.rs:26:30:26:40 | ...::new | provenance | MaD:4 Sink:MaD:4 |
23-
| test_cipher.rs:26:66:26:71 | const4 [&ref] | test_cipher.rs:26:42:26:72 | ...::from_slice(...) [&ref] | provenance | MaD:9 |
31+
| test_cipher.rs:26:66:26:71 | const4 [&ref] | test_cipher.rs:26:42:26:72 | ...::from_slice(...) [&ref] | provenance | MaD:10 |
2432
| test_cipher.rs:29:9:29:14 | const5 [&ref] | test_cipher.rs:30:95:30:100 | const5 [&ref] | provenance | |
2533
| test_cipher.rs:29:28:29:36 | &... [&ref] | test_cipher.rs:29:9:29:14 | const5 [&ref] | provenance | |
2634
| test_cipher.rs:29:29:29:36 | [0u8; 16] | test_cipher.rs:29:28:29:36 | &... [&ref] | provenance | |
2735
| test_cipher.rs:30:72:30:101 | ...::from_slice(...) [&ref] | test_cipher.rs:30:30:30:40 | ...::new | provenance | MaD:5 Sink:MaD:5 |
28-
| test_cipher.rs:30:95:30:100 | const5 [&ref] | test_cipher.rs:30:72:30:101 | ...::from_slice(...) [&ref] | provenance | MaD:9 |
36+
| test_cipher.rs:30:95:30:100 | const5 [&ref] | test_cipher.rs:30:72:30:101 | ...::from_slice(...) [&ref] | provenance | MaD:10 |
2937
| test_cipher.rs:37:9:37:14 | const7 | test_cipher.rs:38:74:38:79 | const7 | provenance | |
3038
| test_cipher.rs:37:27:37:74 | [...] | test_cipher.rs:37:9:37:14 | const7 | provenance | |
3139
| test_cipher.rs:38:49:38:80 | ...::from_slice(...) [&ref] | test_cipher.rs:38:30:38:47 | ...::new | provenance | MaD:3 Sink:MaD:3 |
32-
| test_cipher.rs:38:73:38:79 | &const7 [&ref] | test_cipher.rs:38:49:38:80 | ...::from_slice(...) [&ref] | provenance | MaD:9 |
40+
| test_cipher.rs:38:73:38:79 | &const7 [&ref] | test_cipher.rs:38:49:38:80 | ...::from_slice(...) [&ref] | provenance | MaD:10 |
3341
| test_cipher.rs:38:74:38:79 | const7 | test_cipher.rs:38:73:38:79 | &const7 [&ref] | provenance | |
3442
| test_cipher.rs:41:9:41:14 | const8 [&ref] | test_cipher.rs:42:73:42:78 | const8 [&ref] | provenance | |
3543
| test_cipher.rs:41:28:41:76 | &... [&ref] | test_cipher.rs:41:9:41:14 | const8 [&ref] | provenance | |
3644
| test_cipher.rs:41:29:41:76 | [...] | test_cipher.rs:41:28:41:76 | &... [&ref] | provenance | |
3745
| test_cipher.rs:42:49:42:79 | ...::from_slice(...) [&ref] | test_cipher.rs:42:30:42:47 | ...::new | provenance | MaD:3 Sink:MaD:3 |
38-
| test_cipher.rs:42:73:42:78 | const8 [&ref] | test_cipher.rs:42:49:42:79 | ...::from_slice(...) [&ref] | provenance | MaD:9 |
46+
| test_cipher.rs:42:73:42:78 | const8 [&ref] | test_cipher.rs:42:49:42:79 | ...::from_slice(...) [&ref] | provenance | MaD:10 |
3947
| test_cipher.rs:50:9:50:15 | const10 [element] | test_cipher.rs:51:75:51:81 | const10 [element] | provenance | |
4048
| test_cipher.rs:50:37:50:52 | ...::zeroed | test_cipher.rs:50:37:50:54 | ...::zeroed(...) [element] | provenance | Src:MaD:7 |
4149
| test_cipher.rs:50:37:50:54 | ...::zeroed(...) [element] | test_cipher.rs:50:9:50:15 | const10 [element] | provenance | |
4250
| test_cipher.rs:51:50:51:82 | ...::from_slice(...) [&ref, element] | test_cipher.rs:51:31:51:48 | ...::new | provenance | MaD:3 Sink:MaD:3 Sink:MaD:3 |
43-
| test_cipher.rs:51:74:51:81 | &const10 [&ref, element] | test_cipher.rs:51:50:51:82 | ...::from_slice(...) [&ref, element] | provenance | MaD:9 |
51+
| test_cipher.rs:51:74:51:81 | &const10 [&ref, element] | test_cipher.rs:51:50:51:82 | ...::from_slice(...) [&ref, element] | provenance | MaD:10 |
4452
| test_cipher.rs:51:75:51:81 | const10 [element] | test_cipher.rs:51:74:51:81 | &const10 [&ref, element] | provenance | |
4553
| test_cipher.rs:73:9:73:14 | const2 [&ref] | test_cipher.rs:74:46:74:51 | const2 [&ref] | provenance | |
4654
| test_cipher.rs:73:18:73:26 | &... [&ref] | test_cipher.rs:73:9:73:14 | const2 [&ref] | provenance | |
@@ -59,9 +67,19 @@ edges
5967
| test_cookie.rs:38:28:38:36 | [0u8; 64] | test_cookie.rs:38:18:38:37 | ...::from(...) | provenance | MaD:8 |
6068
| test_cookie.rs:42:34:42:39 | array2 | test_cookie.rs:42:14:42:32 | ...::from | provenance | MaD:2 Sink:MaD:2 |
6169
| test_cookie.rs:49:9:49:14 | array3 [element] | test_cookie.rs:53:34:53:39 | array3 [element] | provenance | |
62-
| test_cookie.rs:49:23:49:25 | 0u8 | test_cookie.rs:49:23:49:29 | ...::from_elem(...) [element] | provenance | MaD:10 |
70+
| test_cookie.rs:49:23:49:25 | 0u8 | test_cookie.rs:49:23:49:29 | ...::from_elem(...) [element] | provenance | MaD:11 |
6371
| test_cookie.rs:49:23:49:29 | ...::from_elem(...) [element] | test_cookie.rs:49:9:49:14 | array3 [element] | provenance | |
6472
| test_cookie.rs:53:34:53:39 | array3 [element] | test_cookie.rs:53:14:53:32 | ...::from | provenance | MaD:2 Sink:MaD:2 |
73+
| test_heuristic.rs:44:9:44:16 | const_iv [&ref] | test_heuristic.rs:45:41:45:48 | const_iv | provenance | |
74+
| test_heuristic.rs:44:30:44:38 | &... [&ref] | test_heuristic.rs:44:9:44:16 | const_iv [&ref] | provenance | |
75+
| test_heuristic.rs:44:31:44:38 | [0u8; 16] | test_heuristic.rs:44:30:44:38 | &... [&ref] | provenance | |
76+
| test_heuristic.rs:64:20:64:27 | [0u8; 16] | test_heuristic.rs:64:19:64:27 | &... | provenance | |
77+
| test_heuristic.rs:65:31:65:38 | [0u8; 16] | test_heuristic.rs:65:30:65:38 | &... | provenance | |
78+
| test_heuristic.rs:69:32:69:32 | 1 | test_heuristic.rs:69:22:69:32 | ... + ... | provenance | |
79+
| test_heuristic.rs:70:23:70:35 | ... << ... | test_heuristic.rs:70:22:70:62 | ... ^ ... | provenance | |
80+
| test_heuristic.rs:70:34:70:35 | 32 | test_heuristic.rs:70:22:70:62 | ... ^ ... | provenance | |
81+
| test_heuristic.rs:70:34:70:35 | 32 | test_heuristic.rs:70:23:70:35 | ... << ... | provenance | MaD:9 |
82+
| test_heuristic.rs:70:52:70:61 | 0xFFFFFFFF | test_heuristic.rs:70:22:70:62 | ... ^ ... | provenance | |
6583
models
6684
| 1 | Sink: <_ as crypto_common::KeyInit>::new_from_slice; Argument[0]; credentials-key |
6785
| 2 | Sink: <biscotti::crypto::master::Key>::from; Argument[0]; credentials-key |
@@ -71,8 +89,9 @@ models
7189
| 6 | Sink: <cookie::secure::key::Key>::from; Argument[0].Reference; credentials-key |
7290
| 7 | Source: core::mem::zeroed; ReturnValue.Element; constant-source |
7391
| 8 | Summary: <_ as core::convert::From>::from; Argument[0]; ReturnValue; taint |
74-
| 9 | Summary: <generic_array::GenericArray>::from_slice; Argument[0].Reference; ReturnValue.Reference; value |
75-
| 10 | Summary: alloc::vec::from_elem; Argument[0]; ReturnValue.Element; value |
92+
| 9 | Summary: <core::u64 as core::ops::bit::Shl>::shl; Argument[0]; ReturnValue; taint |
93+
| 10 | Summary: <generic_array::GenericArray>::from_slice; Argument[0].Reference; ReturnValue.Reference; value |
94+
| 11 | Summary: alloc::vec::from_elem; Argument[0]; ReturnValue.Element; value |
7695
nodes
7796
| test_cipher.rs:18:9:18:14 | const1 [&ref] | semmle.label | const1 [&ref] |
7897
| test_cipher.rs:18:28:18:36 | &... [&ref] | semmle.label | &... [&ref] |
@@ -136,4 +155,20 @@ nodes
136155
| test_cookie.rs:49:23:49:29 | ...::from_elem(...) [element] | semmle.label | ...::from_elem(...) [element] |
137156
| test_cookie.rs:53:14:53:32 | ...::from | semmle.label | ...::from |
138157
| test_cookie.rs:53:34:53:39 | array3 [element] | semmle.label | array3 [element] |
158+
| test_heuristic.rs:44:9:44:16 | const_iv [&ref] | semmle.label | const_iv [&ref] |
159+
| test_heuristic.rs:44:30:44:38 | &... [&ref] | semmle.label | &... [&ref] |
160+
| test_heuristic.rs:44:31:44:38 | [0u8; 16] | semmle.label | [0u8; 16] |
161+
| test_heuristic.rs:45:41:45:48 | const_iv | semmle.label | const_iv |
162+
| test_heuristic.rs:63:30:63:37 | "secret" | semmle.label | "secret" |
163+
| test_heuristic.rs:64:19:64:27 | &... | semmle.label | &... |
164+
| test_heuristic.rs:64:20:64:27 | [0u8; 16] | semmle.label | [0u8; 16] |
165+
| test_heuristic.rs:65:30:65:38 | &... | semmle.label | &... |
166+
| test_heuristic.rs:65:31:65:38 | [0u8; 16] | semmle.label | [0u8; 16] |
167+
| test_heuristic.rs:67:22:67:22 | 0 | semmle.label | 0 |
168+
| test_heuristic.rs:69:22:69:32 | ... + ... | semmle.label | ... + ... |
169+
| test_heuristic.rs:69:32:69:32 | 1 | semmle.label | 1 |
170+
| test_heuristic.rs:70:22:70:62 | ... ^ ... | semmle.label | ... ^ ... |
171+
| test_heuristic.rs:70:23:70:35 | ... << ... | semmle.label | ... << ... |
172+
| test_heuristic.rs:70:34:70:35 | 32 | semmle.label | 32 |
173+
| test_heuristic.rs:70:52:70:61 | 0xFFFFFFFF | semmle.label | 0xFFFFFFFF |
139174
subpaths

‎rust/ql/test/query-tests/security/CWE-798/test_heuristic.rs‎

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
2+
// --- tests ---
3+
4+
fn encrypt_with(plaintext: &str, key: &[u8;16], iv: &[u8;16]) {
5+
// ...
6+
}
7+
8+
fn encrypt2(plaintext: &str, crypto_key: &[u8;16], iv_bytes: &[u8;16]) {
9+
// ...
10+
}
11+
12+
fn database_op(text: &str, primary_key: &str, pivot: &str) {
13+
// note: this one has nothing to do with encryption, but has
14+
// `key` and `iv` contained within the parameter names.
15+
}
16+
17+
struct MyCryptor {
18+
}
19+
20+
impl MyCryptor {
21+
fn new(password: &str) -> MyCryptor {
22+
MyCryptor { }
23+
}
24+
25+
fn set_nonce(&self, nonce: &[u8;16]) {
26+
// ...
27+
}
28+
29+
fn encrypt(&self, plaintext: &str, salt: &[u8;16]) {
30+
// ...
31+
}
32+
33+
fn set_salt_u64(&self, salt: u64) {
34+
// ...
35+
}
36+
}
37+
38+
fn test(var_string: &str, var_data: &[u8;16], var_u64: u64) {
39+
encrypt_with("plaintext", var_data, var_data);
40+
41+
let const_key: &[u8;16] = &[0u8;16]; // $ MISSING: Alert[rust/hard-coded-cryptographic-value]
42+
encrypt_with("plaintext", const_key, var_data); // $ MISSING: Sink
43+
44+
let const_iv: &[u8;16] = &[0u8;16]; // $ Alert[rust/hard-coded-cryptographic-value]
45+
encrypt_with("plaintext", var_data, const_iv); // $ Sink
46+
47+
encrypt2("plaintext", var_data, var_data);
48+
49+
let const_key2: &[u8;16] = &[1u8;16]; // $ MISSING: Alert[rust/hard-coded-cryptographic-value]
50+
encrypt2("plaintext", const_key2, var_data); // $ MISSING: Sink
51+
52+
let const_iv: &[u8;16] = &[1u8;16]; // $ MISSING: Alert[rust/hard-coded-cryptographic-value]
53+
encrypt2("plaintext", var_data, const_iv); // $ MISSING: Sink
54+
55+
let const_key_str = "primary_key";
56+
let const_pivot_str = "pivot";
57+
database_op("text", const_key_str, const_pivot_str);
58+
59+
let mc1 = MyCryptor::new(var_string);
60+
mc1.set_nonce(var_data);
61+
mc1.encrypt("plaintext", var_data);
62+
63+
let mc2 = MyCryptor::new("secret"); // $ Alert[rust/hard-coded-cryptographic-value]
64+
mc2.set_nonce(&[0u8;16]); // $ Alert[rust/hard-coded-cryptographic-value]
65+
mc2.encrypt("plaintext", &[0u8;16]); // $ Alert[rust/hard-coded-cryptographic-value]
66+
67+
mc2.set_salt_u64(0); // $ Alert[rust/hard-coded-cryptographic-value]
68+
mc2.set_salt_u64(var_u64);
69+
mc2.set_salt_u64(var_u64 + 1); // $ SPURIOUS: Alert[rust/hard-coded-cryptographic-value]
70+
mc2.set_salt_u64((var_u64 << 32) ^ (var_u64 & 0xFFFFFFFF)); // $ SPURIOUS: Alert[rust/hard-coded-cryptographic-value]
71+
}

0 commit comments

Comments
 (0)