False positive: `java/tainted-arithmetic` flags bounds-checking arithmetic expressions

[MarkLee131]

Description of the false positive

The java/tainted-arithmetic query (CWE-190/CWE-191) reports false positives when a tainted value is used in an arithmetic expression that is itself a bounds check. A typical example:

if (off + len > array.length) {
    throw new IndexOutOfBoundsException();
}

Here off + len is flagged as "arithmetic on user-provided value, potentially causing overflow." However, the entire expression is a defensive bounds check. The addition is not used for indexing or allocation. It is compared against a known bound, and the method throws if the check fails.

Root cause

ArithmeticCommon.qll defines overflowSink as any AddExpr/MulExpr where an operand is tainted. The existing overflowBarrier recognizes comparison guards on the tainted variable itself (via sizeGuardLessThan), but it does not handle the case where the arithmetic expression containing the tainted variable appears directly as an operand of a comparison.

For example, if (data < MAX) is recognized as a guard on data, but if (data + offset > limit) is not. In the second case, the addition is the bounds check, not a vulnerable computation.

Code samples or links to source code

Found while scanning Apache Dubbo (branch 3.3). 6 out of 7 findings are false positives. Representative example:

https://github.com/apache/dubbo/blob/3.3/dubbo-common/src/main/java/org/apache/dubbo/common/io/Bytes.java#L397-L410

public static String bytes2hex(byte[] bs, int off, int len) {
    if (off < 0) {
        throw new IndexOutOfBoundsException("offset < 0");
    }
    if (len < 0) {
        throw new IndexOutOfBoundsException("length < 0");
    }
    if (off + len > bs.length) {  // ← off + len flagged as tainted arithmetic
        throw new IndexOutOfBoundsException("offset + length > array length.");
    }
    char[] cs = new char[len * 2];  // ← len * 2 also flagged
    // ...
}

Both off + len (line 404) and len * 2 (line 410) are flagged. The addition at line 404 is purely a bounds check. Even if it overflows, the comparison still catches the invalid state and the method throws. The multiplication at line 410 is a more reasonable finding, but the preceding guards ensure off and len are non-negative and bounded by bs.length, making overflow practically impossible.

---

I am happy to submit a PR to fix and improve this rule. :)