Skip to content

Commit ab1bc85

Browse files
authored
Merge pull request #22053 from geoffw0/arith
Rust: Fix FPs in rust/hard-coded-cryptographic-value
2 parents f4d8358 + 1f4ae86 commit ab1bc85

4 files changed

Lines changed: 146 additions & 72 deletions

File tree

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

Lines changed: 43 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -62,24 +62,34 @@ module HardcodedCryptographicValue {
6262
abstract class Barrier extends DataFlow::Node { }
6363

6464
/**
65-
* A literal, considered as a flow source.
65+
* Holds if `e` is a literal or a combination of literals that is constant.
6666
*/
67-
private class LiteralSource extends Source {
68-
LiteralSource() { this.asExpr() instanceof LiteralExpr }
67+
private predicate isConstant(Expr e) {
68+
e instanceof LiteralExpr // e.g. `0`
69+
or
70+
forex(Expr elem | elem = e.(ArrayListExpr).getExpr(_) | isConstant(elem)) // e.g. `[0, 0, 0, 0]`
71+
or
72+
isConstant(e.(ArrayRepeatExpr).getRepeatOperand()) // e.g. `[0; 10]`
73+
or
74+
// e.g. `const MY_CONST: u64 = ...`
75+
// the constant initializer / body is the preferred source location for flow paths, when available.
76+
e = any(Const c).getBody()
77+
or
78+
// e.g. `u64::MAX`
79+
// when the constant initializer is not available as a source location (case above), use the access instead.
80+
e instanceof ConstAccess and
81+
not exists(e.(ConstAccess).getConst().getBody())
82+
or
83+
// e.g. `1 << 4`
84+
isConstant(e.(BinaryExpr).getLhs()) and
85+
isConstant(e.(BinaryExpr).getRhs())
6986
}
7087

7188
/**
72-
* An array initialized from a list of literals, considered as a single flow source. For example:
73-
* ```
74-
* [0, 0, 0, 0]
75-
* [0; 10]
76-
* ```
89+
* A constant, considered as a flow source.
7790
*/
78-
private class ArrayListSource extends Source {
79-
ArrayListSource() {
80-
this.asExpr().(ArrayListExpr).getExpr(_) instanceof LiteralExpr or
81-
this.asExpr().(ArrayRepeatExpr).getRepeatOperand() instanceof LiteralExpr
82-
}
91+
private class ConstantSource extends Source {
92+
ConstantSource() { isConstant(this.asExpr()) }
8393
}
8494

8595
/**
@@ -155,4 +165,24 @@ module HardcodedCryptographicValue {
155165
)
156166
}
157167
}
168+
169+
/**
170+
* An arithmetic or bitwise operation that acts as a barrier.
171+
*
172+
* This prevents false positives where a hard-coded value is combined with
173+
* non-constant data through operations like `+`, `^`, or `+=` (including string concatenation).
174+
*/
175+
private class ArithmeticOperationBarrier extends Barrier {
176+
ArithmeticOperationBarrier() {
177+
// binary operations (e.g. `a + b`, `a ^ b`)
178+
this.asExpr() = any(BinaryArithmeticOperation a).getAnOperand()
179+
or
180+
this.asExpr() = any(BinaryBitwiseOperation a).getAnOperand()
181+
or
182+
// compound assignments (e.g. `a += b`, `a ^= b`)
183+
this.asExpr() = any(AssignArithmeticOperation a).getAnOperand()
184+
or
185+
this.asExpr() = any(AssignBitwiseOperation a).getAnOperand()
186+
}
187+
}
158188
}
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 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.

0 commit comments

Comments
 (0)