mirror of
https://github.com/github/codeql.git
synced 2026-03-31 12:48:17 +02:00
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.
This commit is contained in:
@@ -0,0 +1,4 @@
|
||||
---
|
||||
category: minorAnalysis
|
||||
---
|
||||
* 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.
|
||||
@@ -132,7 +132,21 @@ private predicate inBitwiseAnd(Expr exp) {
|
||||
/** Holds if overflow/underflow is irrelevant for this expression. */
|
||||
predicate overflowIrrelevant(Expr exp) {
|
||||
inBitwiseAnd(exp) or
|
||||
exp.getEnclosingCallable() instanceof HashCodeMethod
|
||||
exp.getEnclosingCallable() instanceof HashCodeMethod or
|
||||
arithmeticUsedInBoundsCheck(exp)
|
||||
}
|
||||
|
||||
/**
|
||||
* Holds if `exp` is an arithmetic expression used directly as an operand of a
|
||||
* comparison, indicating it is part of a bounds check rather than a vulnerable
|
||||
* computation. For example, in `if (off + len > array.length)`, the addition
|
||||
* is the bounds check itself.
|
||||
*/
|
||||
private predicate arithmeticUsedInBoundsCheck(ArithExpr exp) {
|
||||
exists(ComparisonExpr comp |
|
||||
comp.getAnOperand() = exp and
|
||||
comp.getEnclosingStmt() instanceof IfStmt
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -138,4 +138,18 @@ public class ArithmeticTainted {
|
||||
// BAD: may underflow if input data is very small
|
||||
--data;
|
||||
}
|
||||
|
||||
public static void boundsCheckGood(byte[] bs, int off, int len) {
|
||||
// GOOD: arithmetic used directly in a bounds check, not as a computation
|
||||
if (off + len > bs.length) {
|
||||
throw new IndexOutOfBoundsException();
|
||||
}
|
||||
}
|
||||
|
||||
public static void boundsCheckGood2(int[] arr, int offset, int count) {
|
||||
// GOOD: subtraction used directly in a bounds check
|
||||
if (offset - count < 0) {
|
||||
throw new IndexOutOfBoundsException();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user