hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-03-31 04:38:18 +02:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #21608 from MarkLee131/fix/tainted-arithmetic-bounds-check-barrier

Browse Source 
Exclude bounds-check arithmetic from tainted-arithmetic sinks
This commit is contained in:
Owen Mansel-Chan
2026-03-29 22:47:20 +01:00
committed by GitHub
parent 58218ee630 e6adfbca77
commit 898d12b0be
4 changed files with 55 additions and 21 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
java/ql/lib/change-notes/2026-03-28-tainted-arithmetic-bounds-check.md Normal file
View File

@@ -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
java/ql/lib/semmle/code/java/security/ArithmeticCommon.qll
View File

@@ -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
)
}
/**

40
java/ql/test/query-tests/security/CWE-190/semmle/tests/ArithmeticTainted.expected
View File

@@ -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 |

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

@@ -119,6 +119,8 @@ public class ArithmeticTainted {
test2(data);
test3(data);
test4(data);
boundsCheckGood(null, data, 5);
boundsCheckGood2(null, data, 5);
}
}
@@ -138,4 +140,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();
}
}
}