From 0c5e89a68eff355ca81aa710827194f501046496 Mon Sep 17 00:00:00 2001 From: MarkLee131 Date: Sat, 28 Mar 2026 17:39:40 +0800 Subject: [PATCH 1/4] 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. --- ...2026-03-28-tainted-arithmetic-bounds-check.md | 4 ++++ .../code/java/security/ArithmeticCommon.qll | 16 +++++++++++++++- .../CWE-190/semmle/tests/ArithmeticTainted.java | 14 ++++++++++++++ 3 files changed, 33 insertions(+), 1 deletion(-) create mode 100644 java/ql/lib/change-notes/2026-03-28-tainted-arithmetic-bounds-check.md diff --git a/java/ql/lib/change-notes/2026-03-28-tainted-arithmetic-bounds-check.md b/java/ql/lib/change-notes/2026-03-28-tainted-arithmetic-bounds-check.md new file mode 100644 index 00000000000..238b1e2978f --- /dev/null +++ b/java/ql/lib/change-notes/2026-03-28-tainted-arithmetic-bounds-check.md @@ -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. diff --git a/java/ql/lib/semmle/code/java/security/ArithmeticCommon.qll b/java/ql/lib/semmle/code/java/security/ArithmeticCommon.qll index 9282e766627..791c1ab7ba0 100644 --- a/java/ql/lib/semmle/code/java/security/ArithmeticCommon.qll +++ b/java/ql/lib/semmle/code/java/security/ArithmeticCommon.qll @@ -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 + ) } /** diff --git a/java/ql/test/query-tests/security/CWE-190/semmle/tests/ArithmeticTainted.java b/java/ql/test/query-tests/security/CWE-190/semmle/tests/ArithmeticTainted.java index 5fde69929b2..80a83392873 100644 --- a/java/ql/test/query-tests/security/CWE-190/semmle/tests/ArithmeticTainted.java +++ b/java/ql/test/query-tests/security/CWE-190/semmle/tests/ArithmeticTainted.java @@ -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(); + } + } } From f5cfc5e282d73e4f89eb2377aa9e7966ed54d8e1 Mon Sep 17 00:00:00 2001 From: Kaixuan Li Date: Sun, 29 Mar 2026 10:25:10 +0800 Subject: [PATCH 2/4] Update java/ql/test/query-tests/security/CWE-190/semmle/tests/ArithmeticTainted.java Co-authored-by: Owen Mansel-Chan <62447351+owen-mc@users.noreply.github.com> --- .../security/CWE-190/semmle/tests/ArithmeticTainted.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/java/ql/test/query-tests/security/CWE-190/semmle/tests/ArithmeticTainted.java b/java/ql/test/query-tests/security/CWE-190/semmle/tests/ArithmeticTainted.java index 80a83392873..04020aac31f 100644 --- a/java/ql/test/query-tests/security/CWE-190/semmle/tests/ArithmeticTainted.java +++ b/java/ql/test/query-tests/security/CWE-190/semmle/tests/ArithmeticTainted.java @@ -119,6 +119,8 @@ public class ArithmeticTainted { test2(data); test3(data); test4(data); + boundsCheckGood(null, data, 5); + boundsCheckGood2(null, data, 5); } } From b595a70384d4e118dd5fdb94afbb73e27474fb40 Mon Sep 17 00:00:00 2001 From: Kaixuan Li Date: Sun, 29 Mar 2026 11:45:27 +0800 Subject: [PATCH 3/4] Update java/ql/lib/change-notes/2026-03-28-tainted-arithmetic-bounds-check.md Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../change-notes/2026-03-28-tainted-arithmetic-bounds-check.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/java/ql/lib/change-notes/2026-03-28-tainted-arithmetic-bounds-check.md b/java/ql/lib/change-notes/2026-03-28-tainted-arithmetic-bounds-check.md index 238b1e2978f..0688815c822 100644 --- a/java/ql/lib/change-notes/2026-03-28-tainted-arithmetic-bounds-check.md +++ b/java/ql/lib/change-notes/2026-03-28-tainted-arithmetic-bounds-check.md @@ -1,4 +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. +* 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. From e6adfbca77eeb010bd0a8fbb4df5857d8829f177 Mon Sep 17 00:00:00 2001 From: MarkLee131 Date: Sun, 29 Mar 2026 11:53:06 +0800 Subject: [PATCH 4/4] Address review: update QLDoc comment and fix expected test output - Clarify that arithmeticUsedInBoundsCheck applies to if-condition comparisons, not all comparisons - Update expected test line numbers to reflect added test calls --- .../code/java/security/ArithmeticCommon.qll | 6 +-- .../semmle/tests/ArithmeticTainted.expected | 40 +++++++++---------- 2 files changed, 23 insertions(+), 23 deletions(-) diff --git a/java/ql/lib/semmle/code/java/security/ArithmeticCommon.qll b/java/ql/lib/semmle/code/java/security/ArithmeticCommon.qll index 791c1ab7ba0..42b7ec7b02d 100644 --- a/java/ql/lib/semmle/code/java/security/ArithmeticCommon.qll +++ b/java/ql/lib/semmle/code/java/security/ArithmeticCommon.qll @@ -138,9 +138,9 @@ predicate overflowIrrelevant(Expr 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. + * 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 | diff --git a/java/ql/test/query-tests/security/CWE-190/semmle/tests/ArithmeticTainted.expected b/java/ql/test/query-tests/security/CWE-190/semmle/tests/ArithmeticTainted.expected index f7277e3079c..a39920b9065 100644 --- a/java/ql/test/query-tests/security/CWE-190/semmle/tests/ArithmeticTainted.expected +++ b/java/ql/test/query-tests/security/CWE-190/semmle/tests/ArithmeticTainted.expected @@ -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 | | @@ -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 | | @@ -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 |