github / github/codeql

False Positive: IterableIterator.ql reports classes whose `hasNext()` still reliably disables iteration.

Open
#21,550 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
CodeQL
Stars
10.1k
Forks
2.1k
Avg merge
2d 15h
Merged PRs (30d)
141

Description

# False Positive: IterableIterator.ql reports classes whose `hasNext()` still reliably disables iteration.

Version
codeql 2.24.3

## Checker
- Checker id: `Language Abuse/IterableIterator.ql`
- Checker description: This checker detects classes that implement Iterable by returning themselves as the Iterator but lack a guard to prevent multiple concurrent iterations.

## Description of the false positive
These classes do return `this` from `iterator()`, but `hasNext()` still deterministically returns `false`, which is exactly the built-in guard that keeps iteration from proceeding. The refactoring only changes how that `false` result is computed.

## Affected test cases
### `NegCase6_Var2.java`
`hasNext()` still disables reuse of the iterator instance in practice, so this should not be reported as an unsafe self-iterable.

```java
// A concrete class that implements Iterable, returns "this" in iterator(), and has hasNext() returning false should not be flagged.
package scensct.var.neg;

import java.util.Iterator;

public class NegCase6_Var2 implements Iterable, Iterator { // [REPORTED LINE]

public Iterator iterator() {
return this;
}

private boolean neverHasNext() {
return false;
}

public boolean hasNext() {
return neverHasNext();
}

public Double next() {
return 0.0;
}
}
```

### `NegCase6_Var4.java`
The iteration guard is still present even though the control flow is slightly different.

```java
// A concrete class that implements Iterable, returns "this" in iterator(), and has hasNext() returning false should not be flagged.
package scensct.var.neg;

import java.util.Iterator;

public class NegCase6_Var4 implements Iterable, Iterator { // [REPORTED LINE]
private final boolean NO_MORE = false;

public Iterator iterator() {
return this;
}

public boolean hasNext() {
for (int i = 0; i < 1; i++) {
// loop does nothing
}
return NO_MORE;
}

public Double next() {
return 0.0;
}
}
```

## Cause analysis
The query appears too literal about what counts as a valid guard. Once `hasNext()` returns `false` via a helper or a constant field instead of a bare literal, the class is still reported.

That is overly rigid. The safety property here is semantic: iteration is disabled, regardless of whether `false` is returned directly or indirectly.

## References
None known.

Contributor guide

Open the contributing guide

Research direction

Start with the Language Abuse/IterableIterator.ql checker and the affected NegCase6_Var2.java and NegCase6_Var4.java test cases. Trace how hasNext() is recognized when it returns false through a helper or constant, then update the checker tests so these cases are no longer reported while the unsafe self-iterable cases remain covered.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
security, testing-qa
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.