Skip to content

Exclude bounds-check arithmetic from tainted-arithmetic sinks #21608

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
MarkLee131 wants to merge 5 commits into
base: main
Choose a base branch
from
+55 −21
Open

Exclude bounds-check arithmetic from tainted-arithmetic sinks #21608

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0c5e89a
Exclude bounds-check arithmetic from tainted-arithmetic sinks
MarkLee131 Mar 28, 2026
f5cfc5e
Update java/ql/test/query-tests/security/CWE-190/semmle/tests/Arithme…
MarkLee131 Mar 29, 2026
938039d
Merge branch 'main' into fix/tainted-arithmetic-bounds-check-barrier
MarkLee131 Mar 29, 2026
b595a70
Update java/ql/lib/change-notes/2026-03-28-tainted-arithmetic-bounds-…
MarkLee131 Mar 29, 2026
e6adfbc
Address review: update QLDoc comment and fix expected test output
MarkLee131 Mar 29, 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
4 changes: 4 additions & 0 deletions java/ql/lib/change-notes/2026-03-28-tainted-arithmetic-bounds-check.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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 `if`-condition 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.
16 changes: 15 additions & 1 deletion java/ql/lib/semmle/code/java/security/ArithmeticCommon.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 in an `if`-condition, 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
)
}

/**
Expand Down
40 changes: 20 additions & 20 deletions java/ql/test/query-tests/security/CWE-190/semmle/tests/ArithmeticTainted.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@
| ArithmeticTainted.java:50:17:50:24 | ... + ... | ArithmeticTainted.java:17:46:17:54 | System.in : InputStream | ArithmeticTainted.java:50:17:50:20 | data | This arithmetic expression depends on a $@, potentially causing an overflow. | ArithmeticTainted.java:17:46:17:54 | System.in | user-provided value |
| ArithmeticTainted.java:71:17:71:27 | ... + ... | ArithmeticTainted.java:17:46:17:54 | System.in : InputStream | ArithmeticTainted.java:71:17:71:23 | herring | This arithmetic expression depends on a $@, potentially causing an overflow. | ArithmeticTainted.java:17:46:17:54 | System.in | user-provided value |
| ArithmeticTainted.java:95:37:95:46 | ... + ... | ArithmeticTainted.java:17:46:17:54 | System.in : InputStream | ArithmeticTainted.java:95:37:95:40 | data | This arithmetic expression depends on a $@, potentially causing an overflow. | ArithmeticTainted.java:17:46:17:54 | System.in | user-provided value |
| ArithmeticTainted.java:127:3:127:8 | ...++ | ArithmeticTainted.java:17:46:17:54 | System.in : InputStream | ArithmeticTainted.java:127:3:127:6 | data | This arithmetic expression depends on a $@, potentially causing an overflow. | ArithmeticTainted.java:17:46:17:54 | System.in | user-provided value |
| ArithmeticTainted.java:131:3:131:8 | ++... | ArithmeticTainted.java:17:46:17:54 | System.in : InputStream | ArithmeticTainted.java:131:5:131:8 | data | This arithmetic expression depends on a $@, potentially causing an overflow. | ArithmeticTainted.java:17:46:17:54 | System.in | user-provided value |
| ArithmeticTainted.java:135:3:135:8 | ...-- | ArithmeticTainted.java:17:46:17:54 | System.in : InputStream | ArithmeticTainted.java:135:3:135:6 | data | This arithmetic expression depends on a $@, potentially causing an underflow. | ArithmeticTainted.java:17:46:17:54 | System.in | user-provided value |
| ArithmeticTainted.java:139:3:139:8 | --... | ArithmeticTainted.java:17:46:17:54 | System.in : InputStream | ArithmeticTainted.java:139:5:139:8 | data | This arithmetic expression depends on a $@, potentially causing an underflow. | ArithmeticTainted.java:17:46:17:54 | System.in | user-provided value |
| ArithmeticTainted.java:129:3:129:8 | ...++ | ArithmeticTainted.java:17:46:17:54 | System.in : InputStream | ArithmeticTainted.java:129:3:129:6 | data | This arithmetic expression depends on a $@, potentially causing an overflow. | ArithmeticTainted.java:17:46:17:54 | System.in | user-provided value |
| ArithmeticTainted.java:133:3:133:8 | ++... | ArithmeticTainted.java:17:46:17:54 | System.in : InputStream | ArithmeticTainted.java:133:5:133:8 | data | This arithmetic expression depends on a $@, potentially causing an overflow. | ArithmeticTainted.java:17:46:17:54 | System.in | user-provided value |
| ArithmeticTainted.java:137:3:137:8 | ...-- | ArithmeticTainted.java:17:46:17:54 | System.in : InputStream | ArithmeticTainted.java:137:3:137:6 | data | This arithmetic expression depends on a $@, potentially causing an underflow. | ArithmeticTainted.java:17:46:17:54 | System.in | user-provided value |
| ArithmeticTainted.java:141:3:141:8 | --... | ArithmeticTainted.java:17:46:17:54 | System.in : InputStream | ArithmeticTainted.java:141:5:141:8 | data | This arithmetic expression depends on a $@, potentially causing an underflow. | ArithmeticTainted.java:17:46:17:54 | System.in | user-provided value |
edges
| ArithmeticTainted.java:17:24:17:64 | new InputStreamReader(...) : InputStreamReader | ArithmeticTainted.java:18:40:18:56 | readerInputStream : InputStreamReader | provenance | |
| ArithmeticTainted.java:17:24:17:64 | new InputStreamReader(...) : InputStreamReader | ArithmeticTainted.java:18:40:18:56 | readerInputStream : InputStreamReader | provenance | |
Expand Down Expand Up @@ -38,14 +38,14 @@ edges
| ArithmeticTainted.java:66:18:66:24 | tainted : Holder [dat] : Number | ArithmeticTainted.java:66:18:66:34 | getData(...) : Number | provenance | |
| ArithmeticTainted.java:66:18:66:24 | tainted : Holder [dat] : Number | Holder.java:16:13:16:19 | parameter this : Holder [dat] : Number | provenance | |
| ArithmeticTainted.java:66:18:66:34 | getData(...) : Number | ArithmeticTainted.java:71:17:71:23 | herring | provenance | |
| ArithmeticTainted.java:118:9:118:12 | data : Number | ArithmeticTainted.java:125:26:125:33 | data : Number | provenance | |
| ArithmeticTainted.java:119:10:119:13 | data : Number | ArithmeticTainted.java:129:27:129:34 | data : Number | provenance | |
| ArithmeticTainted.java:120:10:120:13 | data : Number | ArithmeticTainted.java:133:27:133:34 | data : Number | provenance | |
| ArithmeticTainted.java:121:10:121:13 | data : Number | ArithmeticTainted.java:137:27:137:34 | data : Number | provenance | |
| ArithmeticTainted.java:125:26:125:33 | data : Number | ArithmeticTainted.java:127:3:127:6 | data | provenance | |
| ArithmeticTainted.java:129:27:129:34 | data : Number | ArithmeticTainted.java:131:5:131:8 | data | provenance | |
| ArithmeticTainted.java:133:27:133:34 | data : Number | ArithmeticTainted.java:135:3:135:6 | data | provenance | |
| ArithmeticTainted.java:137:27:137:34 | data : Number | ArithmeticTainted.java:139:5:139:8 | data | provenance | |
| ArithmeticTainted.java:118:9:118:12 | data : Number | ArithmeticTainted.java:127:26:127:33 | data : Number | provenance | |
| ArithmeticTainted.java:119:10:119:13 | data : Number | ArithmeticTainted.java:131:27:131:34 | data : Number | provenance | |
| ArithmeticTainted.java:120:10:120:13 | data : Number | ArithmeticTainted.java:135:27:135:34 | data : Number | provenance | |
| ArithmeticTainted.java:121:10:121:13 | data : Number | ArithmeticTainted.java:139:27:139:34 | data : Number | provenance | |
| ArithmeticTainted.java:127:26:127:33 | data : Number | ArithmeticTainted.java:129:3:129:6 | data | provenance | |
| ArithmeticTainted.java:131:27:131:34 | data : Number | ArithmeticTainted.java:133:5:133:8 | data | provenance | |
| ArithmeticTainted.java:135:27:135:34 | data : Number | ArithmeticTainted.java:137:3:137:6 | data | provenance | |
| ArithmeticTainted.java:139:27:139:34 | data : Number | ArithmeticTainted.java:141:5:141:8 | data | provenance | |
| Holder.java:12:22:12:26 | d : Number | Holder.java:13:9:13:9 | d : Number | provenance | |
| Holder.java:13:3:13:5 | this <.field> [post update] : Holder [dat] : Number | Holder.java:12:14:12:20 | parameter this [Return] : Holder [dat] : Number | provenance | |
| Holder.java:13:9:13:9 | d : Number | Holder.java:13:3:13:5 | this <.field> [post update] : Holder [dat] : Number | provenance | |
Expand Down Expand Up @@ -86,14 +86,14 @@ nodes
| ArithmeticTainted.java:119:10:119:13 | data : Number | semmle.label | data : Number |
| ArithmeticTainted.java:120:10:120:13 | data : Number | semmle.label | data : Number |
| ArithmeticTainted.java:121:10:121:13 | data : Number | semmle.label | data : Number |
| ArithmeticTainted.java:125:26:125:33 | data : Number | semmle.label | data : Number |
| ArithmeticTainted.java:127:3:127:6 | data | semmle.label | data |
| ArithmeticTainted.java:129:27:129:34 | data : Number | semmle.label | data : Number |
| ArithmeticTainted.java:131:5:131:8 | data | semmle.label | data |
| ArithmeticTainted.java:133:27:133:34 | data : Number | semmle.label | data : Number |
| ArithmeticTainted.java:135:3:135:6 | data | semmle.label | data |
| ArithmeticTainted.java:137:27:137:34 | data : Number | semmle.label | data : Number |
| ArithmeticTainted.java:139:5:139:8 | data | semmle.label | data |
| ArithmeticTainted.java:127:26:127:33 | data : Number | semmle.label | data : Number |
| ArithmeticTainted.java:129:3:129:6 | data | semmle.label | data |
| ArithmeticTainted.java:131:27:131:34 | data : Number | semmle.label | data : Number |
| ArithmeticTainted.java:133:5:133:8 | data | semmle.label | data |
| ArithmeticTainted.java:135:27:135:34 | data : Number | semmle.label | data : Number |
| ArithmeticTainted.java:137:3:137:6 | data | semmle.label | data |
| ArithmeticTainted.java:139:27:139:34 | data : Number | semmle.label | data : Number |
| ArithmeticTainted.java:141:5:141:8 | data | semmle.label | data |
| Holder.java:12:14:12:20 | parameter this [Return] : Holder [dat] : Number | semmle.label | parameter this [Return] : Holder [dat] : Number |
| Holder.java:12:22:12:26 | d : Number | semmle.label | d : Number |
| Holder.java:13:3:13:5 | this <.field> [post update] : Holder [dat] : Number | semmle.label | this <.field> [post update] : Holder [dat] : Number |
Expand Down
16 changes: 16 additions & 0 deletions java/ql/test/query-tests/security/CWE-190/semmle/tests/ArithmeticTainted.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,8 @@ public void main(String[] args) {
test2(data);
test3(data);
test4(data);
boundsCheckGood(null, data, 5);
boundsCheckGood2(null, data, 5);
}
}

Expand All @@ -138,4 +140,18 @@ public static void test4(int data) {
// 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();
}
}
}
Loading