Skip to content

Commit 0c5e89a

Browse files
committed
Exclude bounds-check arithmetic from tainted-arithmetic sinks
The java/tainted-arithmetic query now recognizes when an arithmetic expression appears directly as an operand of a comparison (e.g., `if (off + len > array.length)`). Such expressions are bounds checks, not vulnerable computations, and are excluded via the existing overflowIrrelevant predicate. Add test cases for bounds-checking patterns that should not be flagged.
1 parent a8b52ac commit 0c5e89a

File tree

3 files changed

+33
-1
lines changed

3 files changed

+33
-1
lines changed
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 `java/tainted-arithmetic` query no longer flags arithmetic expressions that are used directly as an operand of a comparison in bounds-checking patterns. For example, `if (off + len > array.length)` is now recognized as a bounds check rather than a potentially vulnerable computation, reducing false positives.

java/ql/lib/semmle/code/java/security/ArithmeticCommon.qll

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,21 @@ private predicate inBitwiseAnd(Expr exp) {
132132
/** Holds if overflow/underflow is irrelevant for this expression. */
133133
predicate overflowIrrelevant(Expr exp) {
134134
inBitwiseAnd(exp) or
135-
exp.getEnclosingCallable() instanceof HashCodeMethod
135+
exp.getEnclosingCallable() instanceof HashCodeMethod or
136+
arithmeticUsedInBoundsCheck(exp)
137+
}
138+
139+
/**
140+
* Holds if `exp` is an arithmetic expression used directly as an operand of a
141+
* comparison, indicating it is part of a bounds check rather than a vulnerable
142+
* computation. For example, in `if (off + len > array.length)`, the addition
143+
* is the bounds check itself.
144+
*/
145+
private predicate arithmeticUsedInBoundsCheck(ArithExpr exp) {
146+
exists(ComparisonExpr comp |
147+
comp.getAnOperand() = exp and
148+
comp.getEnclosingStmt() instanceof IfStmt
149+
)
136150
}
137151

138152
/**

java/ql/test/query-tests/security/CWE-190/semmle/tests/ArithmeticTainted.java

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,4 +138,18 @@ public static void test4(int data) {
138138
// BAD: may underflow if input data is very small
139139
--data;
140140
}
141+
142+
public static void boundsCheckGood(byte[] bs, int off, int len) {
143+
// GOOD: arithmetic used directly in a bounds check, not as a computation
144+
if (off + len > bs.length) {
145+
throw new IndexOutOfBoundsException();
146+
}
147+
}
148+
149+
public static void boundsCheckGood2(int[] arr, int offset, int count) {
150+
// GOOD: subtraction used directly in a bounds check
151+
if (offset - count < 0) {
152+
throw new IndexOutOfBoundsException();
153+
}
154+
}
141155
}

0 commit comments

Comments
 (0)