Skip to content

Rust: Fix FPs in rust/hard-coded-cryptographic-value #22053

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

Open
geoffw0 wants to merge 7 commits into
base: main
Choose a base branch
from
+81 −52
Open

Rust: Fix FPs in rust/hard-coded-cryptographic-value #22053

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
0c72a2b
Add test cases involving string appends.
geoffw0 Jun 24, 2026
4bda03f
Rust: Make arithmetic operations a barrier for rust/hard-coded-crypto…
geoffw0 Jun 25, 2026
351d495
Rust: Change note.
geoffw0 Jun 25, 2026
4aa53b6
Update rust/ql/lib/codeql/rust/security/HardcodedCryptographicValueEx…
geoffw0 Jun 25, 2026
3403cff
Rust: More-or-less the Copilot suggestion.
geoffw0 Jun 25, 2026
8155ff7
Rust: Add a few more test cases for constants / constant propagation.
geoffw0 Jun 26, 2026
95e030f
Rust: Detect constant accesses and perform very limited constant prop…
geoffw0 Jun 26, 2026
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
49 changes: 36 additions & 13 deletions rust/ql/lib/codeql/rust/security/HardcodedCryptographicValueExtensions.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,24 +62,27 @@ module HardcodedCryptographicValue {
abstract class Barrier extends DataFlow::Node { }

/**
* A literal, considered as a flow source.
* Holds if `e` is a literal or a combination of literals that is constant.
*/
private class LiteralSource extends Source {
LiteralSource() { this.asExpr() instanceof LiteralExpr }
private predicate isConstant(Expr e) {
e instanceof LiteralExpr // e.g. `0`
or
e.(ArrayListExpr).getExpr(_) instanceof LiteralExpr // e.g. `[0, 0, 0, 0]`
or
e.(ArrayRepeatExpr).getRepeatOperand() instanceof LiteralExpr // e.g. `[0; 10]`
or
e instanceof ConstAccess // e.g. `u64::MAX`
or
// e.g. `1 << 4`
isConstant(e.(BinaryExpr).getLhs()) and
isConstant(e.(BinaryExpr).getRhs())
}

/**
* An array initialized from a list of literals, considered as a single flow source. For example:
* ```
* [0, 0, 0, 0]
* [0; 10]
* ```
* A constant, considered as a flow source.
*/
private class ArrayListSource extends Source {
ArrayListSource() {
this.asExpr().(ArrayListExpr).getExpr(_) instanceof LiteralExpr or
this.asExpr().(ArrayRepeatExpr).getRepeatOperand() instanceof LiteralExpr
}
private class ConstantSource extends Source {
ConstantSource() { isConstant(this.asExpr()) }
}

/**
Expand Down Expand Up @@ -155,4 +158,24 @@ module HardcodedCryptographicValue {
)
}
}

/**
* An arithmetic or bitwise operation that acts as a barrier.
*
* This prevents false positives where a hard-coded value is combined with
* non-constant data through operations like `+`, `^`, or `+=` (including string concatenation).
*/
private class ArithmeticOperationBarrier extends Barrier {
ArithmeticOperationBarrier() {
// binary operations (e.g. `a + b`, `a ^ b`)
this.asExpr() = any(BinaryArithmeticOperation a).getAnOperand()
or
this.asExpr() = any(BinaryBitwiseOperation a).getAnOperand()
or
// compound assignments (e.g. `a += b`, `a ^= b`)
this.asExpr() = any(AssignArithmeticOperation a).getAnOperand()
or
this.asExpr() = any(AssignBitwiseOperation a).getAnOperand()
}
}
}
4 changes: 4 additions & 0 deletions ...rc/change-notes/2026-06-25-hard-coded-cryptographic-value-arithmetic-barrier.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* The `rust/hard-coded-cryptographic-value` query now treats arithmetic and bitwise operations, including string append operations, as barriers. This addresses false positive results where hard-coded constants are combined with non-constant data, such as incrementing a nonce or appending variable data to a constant prefix.
61 changes: 24 additions & 37 deletions rust/ql/test/query-tests/security/CWE-798/HardcodedCryptographicValue.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,40 +15,40 @@
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| test_heuristic.rs:71:22:71:27 | ... << ... | test_heuristic.rs:71:22:71:27 | ... << ... | test_heuristic.rs:71:22:71:27 | ... << ... | This hard-coded value is used as $@. | test_heuristic.rs:71:22:71:27 | ... << ... | a salt |
| test_heuristic.rs:72:22:72:29 | ...::MAX | test_heuristic.rs:72:22:72:29 | ...::MAX | test_heuristic.rs:72:22:72:29 | ...::MAX | This hard-coded value is used as $@. | test_heuristic.rs:72:22:72:29 | ...::MAX | a salt |
| test_heuristic.rs:73:22:73:33 | ... / ... | test_heuristic.rs:73:22:73:33 | ... / ... | test_heuristic.rs:73:22:73:33 | ... / ... | This hard-coded value is used as $@. | test_heuristic.rs:73:22:73:33 | ... / ... | a salt |
edges
| test_cipher.rs:18:9:18:14 | const1 [&ref] | test_cipher.rs:19:73:19:78 | const1 [&ref] | provenance | |
| test_cipher.rs:18:28:18:36 | &... [&ref] | test_cipher.rs:18:9:18:14 | const1 [&ref] | provenance | |
| test_cipher.rs:18:29:18:36 | [0u8; 16] | test_cipher.rs:18:28:18:36 | &... [&ref] | provenance | |
| test_cipher.rs:19:49:19:79 | ...::from_slice(...) [&ref] | test_cipher.rs:19:30:19:47 | ...::new | provenance | MaD:3 Sink:MaD:3 |
| test_cipher.rs:19:73:19:78 | const1 [&ref] | test_cipher.rs:19:49:19:79 | ...::from_slice(...) [&ref] | provenance | MaD:17 |
| test_cipher.rs:19:73:19:78 | const1 [&ref] | test_cipher.rs:19:49:19:79 | ...::from_slice(...) [&ref] | provenance | MaD:13 |
| test_cipher.rs:25:9:25:14 | const4 [&ref] | test_cipher.rs:26:66:26:71 | const4 [&ref] | provenance | |
| test_cipher.rs:25:28:25:36 | &... [&ref] | test_cipher.rs:25:9:25:14 | const4 [&ref] | provenance | |
| test_cipher.rs:25:29:25:36 | [0u8; 16] | test_cipher.rs:25:28:25:36 | &... [&ref] | provenance | |
| test_cipher.rs:26:42:26:72 | ...::from_slice(...) [&ref] | test_cipher.rs:26:30:26:40 | ...::new | provenance | MaD:4 Sink:MaD:4 |
| test_cipher.rs:26:66:26:71 | const4 [&ref] | test_cipher.rs:26:42:26:72 | ...::from_slice(...) [&ref] | provenance | MaD:17 |
| test_cipher.rs:26:66:26:71 | const4 [&ref] | test_cipher.rs:26:42:26:72 | ...::from_slice(...) [&ref] | provenance | MaD:13 |
| test_cipher.rs:29:9:29:14 | const5 [&ref] | test_cipher.rs:30:95:30:100 | const5 [&ref] | provenance | |
| test_cipher.rs:29:28:29:36 | &... [&ref] | test_cipher.rs:29:9:29:14 | const5 [&ref] | provenance | |
| test_cipher.rs:29:29:29:36 | [0u8; 16] | test_cipher.rs:29:28:29:36 | &... [&ref] | provenance | |
| test_cipher.rs:30:72:30:101 | ...::from_slice(...) [&ref] | test_cipher.rs:30:30:30:40 | ...::new | provenance | MaD:5 Sink:MaD:5 |
| test_cipher.rs:30:95:30:100 | const5 [&ref] | test_cipher.rs:30:72:30:101 | ...::from_slice(...) [&ref] | provenance | MaD:17 |
| test_cipher.rs:30:95:30:100 | const5 [&ref] | test_cipher.rs:30:72:30:101 | ...::from_slice(...) [&ref] | provenance | MaD:13 |
| test_cipher.rs:37:9:37:14 | const7 | test_cipher.rs:38:74:38:79 | const7 | provenance | |
| test_cipher.rs:37:27:37:74 | [...] | test_cipher.rs:37:9:37:14 | const7 | provenance | |
| test_cipher.rs:38:49:38:80 | ...::from_slice(...) [&ref] | test_cipher.rs:38:30:38:47 | ...::new | provenance | MaD:3 Sink:MaD:3 |
| test_cipher.rs:38:73:38:79 | &const7 [&ref] | test_cipher.rs:38:49:38:80 | ...::from_slice(...) [&ref] | provenance | MaD:17 |
| test_cipher.rs:38:73:38:79 | &const7 [&ref] | test_cipher.rs:38:49:38:80 | ...::from_slice(...) [&ref] | provenance | MaD:13 |
| test_cipher.rs:38:74:38:79 | const7 | test_cipher.rs:38:73:38:79 | &const7 [&ref] | provenance | |
| test_cipher.rs:41:9:41:14 | const8 [&ref] | test_cipher.rs:42:73:42:78 | const8 [&ref] | provenance | |
| test_cipher.rs:41:28:41:76 | &... [&ref] | test_cipher.rs:41:9:41:14 | const8 [&ref] | provenance | |
| test_cipher.rs:41:29:41:76 | [...] | test_cipher.rs:41:28:41:76 | &... [&ref] | provenance | |
| test_cipher.rs:42:49:42:79 | ...::from_slice(...) [&ref] | test_cipher.rs:42:30:42:47 | ...::new | provenance | MaD:3 Sink:MaD:3 |
| test_cipher.rs:42:73:42:78 | const8 [&ref] | test_cipher.rs:42:49:42:79 | ...::from_slice(...) [&ref] | provenance | MaD:17 |
| test_cipher.rs:42:73:42:78 | const8 [&ref] | test_cipher.rs:42:49:42:79 | ...::from_slice(...) [&ref] | provenance | MaD:13 |
| test_cipher.rs:50:9:50:15 | const10 [element] | test_cipher.rs:51:75:51:81 | const10 [element] | provenance | |
| test_cipher.rs:50:37:50:52 | ...::zeroed | test_cipher.rs:50:37:50:54 | ...::zeroed(...) [element] | provenance | Src:MaD:7 |
| test_cipher.rs:50:37:50:54 | ...::zeroed(...) [element] | test_cipher.rs:50:9:50:15 | const10 [element] | provenance | |
| 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 |
| test_cipher.rs:51:74:51:81 | &const10 [&ref, element] | test_cipher.rs:51:50:51:82 | ...::from_slice(...) [&ref, element] | provenance | MaD:17 |
| test_cipher.rs:51:74:51:81 | &const10 [&ref, element] | test_cipher.rs:51:50:51:82 | ...::from_slice(...) [&ref, element] | provenance | MaD:13 |
| test_cipher.rs:51:75:51:81 | const10 [element] | test_cipher.rs:51:74:51:81 | &const10 [&ref, element] | provenance | |
| test_cipher.rs:73:9:73:14 | const2 [&ref] | test_cipher.rs:74:46:74:51 | const2 [&ref] | provenance | |
| test_cipher.rs:73:18:73:26 | &... [&ref] | test_cipher.rs:73:9:73:14 | const2 [&ref] | provenance | |
Expand All @@ -64,26 +64,21 @@ edges
| test_cookie.rs:22:27:22:32 | array2 | test_cookie.rs:22:26:22:32 | &array2 [&ref] | provenance | |
| test_cookie.rs:38:9:38:14 | array2 | test_cookie.rs:42:34:42:39 | array2 | provenance | |
| test_cookie.rs:38:18:38:37 | ...::from(...) | test_cookie.rs:38:9:38:14 | array2 | provenance | |
| test_cookie.rs:38:28:38:36 | [0u8; 64] | test_cookie.rs:38:18:38:37 | ...::from(...) | provenance | MaD:8 |
| test_cookie.rs:38:28:38:36 | [0u8; 64] | test_cookie.rs:38:18:38:37 | ...::from(...) | provenance | MaD:9 |
| test_cookie.rs:38:28:38:36 | [0u8; 64] | test_cookie.rs:38:18:38:37 | ...::from(...) | provenance | MaD:10 |
| test_cookie.rs:38:28:38:36 | [0u8; 64] | test_cookie.rs:38:18:38:37 | ...::from(...) | provenance | MaD:11 |
| test_cookie.rs:38:28:38:36 | [0u8; 64] | test_cookie.rs:38:18:38:37 | ...::from(...) | provenance | MaD:12 |
| test_cookie.rs:38:28:38:36 | [0u8; 64] | test_cookie.rs:38:18:38:37 | ...::from(...) | provenance | MaD:13 |
| test_cookie.rs:38:28:38:36 | [0u8; 64] | test_cookie.rs:38:18:38:37 | ...::from(...) | provenance | MaD:14 |
| test_cookie.rs:38:28:38:36 | [0u8; 64] | test_cookie.rs:38:18:38:37 | ...::from(...) | provenance | MaD:15 |
| test_cookie.rs:38:28:38:36 | [0u8; 64] | test_cookie.rs:38:18:38:37 | ...::from(...) | provenance | MaD:16 |
| test_cookie.rs:42:34:42:39 | array2 | test_cookie.rs:42:14:42:32 | ...::from | provenance | MaD:2 Sink:MaD:2 |
| test_cookie.rs:49:9:49:14 | array3 [element] | test_cookie.rs:53:34:53:39 | array3 [element] | provenance | |
| test_cookie.rs:49:23:49:25 | 0u8 | test_cookie.rs:49:23:49:29 | ...::from_elem(...) [element] | provenance | MaD:18 |
| test_cookie.rs:49:23:49:25 | 0u8 | test_cookie.rs:49:23:49:29 | ...::from_elem(...) [element] | provenance | MaD:14 |
| test_cookie.rs:49:23:49:29 | ...::from_elem(...) [element] | test_cookie.rs:49:9:49:14 | array3 [element] | provenance | |
| test_cookie.rs:53:34:53:39 | array3 [element] | test_cookie.rs:53:14:53:32 | ...::from | provenance | MaD:2 Sink:MaD:2 |
| test_heuristic.rs:44:9:44:16 | const_iv [&ref] | test_heuristic.rs:45:41:45:48 | const_iv | provenance | |
| test_heuristic.rs:44:30:44:38 | &... [&ref] | test_heuristic.rs:44:9:44:16 | const_iv [&ref] | provenance | |
| test_heuristic.rs:44:31:44:38 | [0u8; 16] | test_heuristic.rs:44:30:44:38 | &... [&ref] | provenance | |
| test_heuristic.rs:64:20:64:27 | [0u8; 16] | test_heuristic.rs:64:19:64:27 | &... | provenance | |
| test_heuristic.rs:65:31:65:38 | [0u8; 16] | test_heuristic.rs:65:30:65:38 | &... | provenance | |
| test_heuristic.rs:69:32:69:32 | 1 | test_heuristic.rs:69:22:69:32 | ... + ... | provenance | MaD:8 |
| test_heuristic.rs:70:23:70:35 | ... << ... | test_heuristic.rs:70:22:70:62 | ... ^ ... | provenance | MaD:10 |
| test_heuristic.rs:70:34:70:35 | 32 | test_heuristic.rs:70:23:70:35 | ... << ... | provenance | MaD:11 |
| test_heuristic.rs:70:41:70:61 | ... & ... | test_heuristic.rs:70:22:70:62 | ... ^ ... | provenance | MaD:10 |
| test_heuristic.rs:70:52:70:61 | 0xFFFFFFFF | test_heuristic.rs:70:41:70:61 | ... & ... | provenance | MaD:9 |
models
| 1 | Sink: <_ as crypto_common::KeyInit>::new_from_slice; Argument[0]; credentials-key |
| 2 | Sink: <biscotti::crypto::master::Key>::from; Argument[0]; credentials-key |
Expand All @@ -92,17 +87,13 @@ models
| 5 | Sink: <cipher::stream_wrapper::StreamCipherCoreWrapper as crypto_common::KeyIvInit>::new; Argument[1]; credentials-iv |
| 6 | Sink: <cookie::secure::key::Key>::from; Argument[0].Reference; credentials-key |
| 7 | Source: core::mem::zeroed; ReturnValue.Element; constant-source |
| 8 | Summary: <_ as core::ops::arith::Add>::add; Argument[self,0]; ReturnValue; taint |
| 9 | Summary: <_ as core::ops::bit::BitAnd>::bitand; Argument[self,0]; ReturnValue; taint |
| 10 | Summary: <_ as core::ops::bit::BitXor>::bitxor; Argument[self,0]; ReturnValue; taint |
| 11 | Summary: <_ as core::ops::bit::Shl>::shl; Argument[self,0]; ReturnValue; taint |
| 12 | Summary: <alloc::vec::Vec as core::convert::From>::from; Argument[0].Field[alloc::borrow::Cow::Owned(0)]; ReturnValue; value |
| 13 | Summary: <alloc::vec::Vec as core::convert::From>::from; Argument[0].Field[alloc::bstr::ByteString(0)]; ReturnValue; value |
| 14 | Summary: <alloc::vec::Vec as core::convert::From>::from; Argument[0].Field[alloc::collections::binary_heap::BinaryHeap::data]; ReturnValue; value |
| 15 | Summary: <alloc::vec::Vec as core::convert::From>::from; Argument[0].Field[alloc::string::String::vec]; ReturnValue; value |
| 16 | Summary: <alloc::vec::Vec as core::convert::From>::from; Argument[0]; ReturnValue; taint |
| 17 | Summary: <generic_array::GenericArray>::from_slice; Argument[0].Reference; ReturnValue.Reference; value |
| 18 | Summary: alloc::vec::from_elem; Argument[0]; ReturnValue.Element; value |
| 8 | Summary: <alloc::vec::Vec as core::convert::From>::from; Argument[0].Field[alloc::borrow::Cow::Owned(0)]; ReturnValue; value |
| 9 | Summary: <alloc::vec::Vec as core::convert::From>::from; Argument[0].Field[alloc::bstr::ByteString(0)]; ReturnValue; value |
| 10 | Summary: <alloc::vec::Vec as core::convert::From>::from; Argument[0].Field[alloc::collections::binary_heap::BinaryHeap::data]; ReturnValue; value |
| 11 | Summary: <alloc::vec::Vec as core::convert::From>::from; Argument[0].Field[alloc::string::String::vec]; ReturnValue; value |
| 12 | Summary: <alloc::vec::Vec as core::convert::From>::from; Argument[0]; ReturnValue; taint |
| 13 | Summary: <generic_array::GenericArray>::from_slice; Argument[0].Reference; ReturnValue.Reference; value |
| 14 | Summary: alloc::vec::from_elem; Argument[0]; ReturnValue.Element; value |
nodes
| test_cipher.rs:18:9:18:14 | const1 [&ref] | semmle.label | const1 [&ref] |
| test_cipher.rs:18:28:18:36 | &... [&ref] | semmle.label | &... [&ref] |
Expand Down Expand Up @@ -176,11 +167,7 @@ nodes
| test_heuristic.rs:65:30:65:38 | &... | semmle.label | &... |
| test_heuristic.rs:65:31:65:38 | [0u8; 16] | semmle.label | [0u8; 16] |
| test_heuristic.rs:67:22:67:22 | 0 | semmle.label | 0 |
| test_heuristic.rs:69:22:69:32 | ... + ... | semmle.label | ... + ... |
| test_heuristic.rs:69:32:69:32 | 1 | semmle.label | 1 |
| test_heuristic.rs:70:22:70:62 | ... ^ ... | semmle.label | ... ^ ... |
| test_heuristic.rs:70:23:70:35 | ... << ... | semmle.label | ... << ... |
| test_heuristic.rs:70:34:70:35 | 32 | semmle.label | 32 |
| test_heuristic.rs:70:41:70:61 | ... & ... | semmle.label | ... & ... |
| test_heuristic.rs:70:52:70:61 | 0xFFFFFFFF | semmle.label | 0xFFFFFFFF |
| test_heuristic.rs:71:22:71:27 | ... << ... | semmle.label | ... << ... |
| test_heuristic.rs:72:22:72:29 | ...::MAX | semmle.label | ...::MAX |
| test_heuristic.rs:73:22:73:33 | ... / ... | semmle.label | ... / ... |
subpaths
19 changes: 17 additions & 2 deletions rust/ql/test/query-tests/security/CWE-798/test_heuristic.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,21 @@ fn test(var_string: &str, var_data: &[u8;16], var_u64: u64) {

mc2.set_salt_u64(0); // $ Alert[rust/hard-coded-cryptographic-value]
mc2.set_salt_u64(var_u64);
mc2.set_salt_u64(var_u64 + 1); // $ SPURIOUS: Alert[rust/hard-coded-cryptographic-value]
mc2.set_salt_u64((var_u64 << 32) ^ (var_u64 & 0xFFFFFFFF)); // $ SPURIOUS: Alert[rust/hard-coded-cryptographic-value]
mc2.set_salt_u64(var_u64 + 1);
mc2.set_salt_u64((var_u64 << 32) ^ (var_u64 & 0xFFFFFFFF));
mc2.set_salt_u64(1 << 4); // $ Alert[rust/hard-coded-cryptographic-value]
mc2.set_salt_u64(u64::MAX); // $ Alert[rust/hard-coded-cryptographic-value]
mc2.set_salt_u64(u64::MAX / 4); // $ Alert[rust/hard-coded-cryptographic-value]

let mut key1 = "foo".to_string(); // $ MISSING: Alert[rust/hard-coded-cryptographic-value]
key1 += "bar"; // $ MISSING: Alert[rust/hard-coded-cryptographic-value]
let _ = MyCryptor::new(&key1);

let mut key2 = "foo".to_string();
key2 += var_string;
let _ = MyCryptor::new(&key2);

let mut key3 = var_string.to_string();
key3 += "bar";
let _ = MyCryptor::new(&key3);
}
Loading