False Negative: IterableIterator.ql misses `iterator() == this` implementations once the guard logic is hidden behind helpers or trivial control flow.
- Vorherrschende Sprache
- CodeQL
- Sterne
- 10.1k
- Forks
- 2.1k
- Ø Merge
- 2 T. 15 Std.
- Gemergte PRs (30 T.)
- 141
Beschreibung
# False Negative: IterableIterator.ql misses `iterator() == this` implementations once the guard logic is hidden behind helpers or trivial control flow.
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 negative
All three samples still implement the same dangerous pattern: `iterator()` returns `this`, the object is also its own `Iterator`, and `hasNext()` does not provide a real guard against repeated or concurrent iteration. The only changes are that `this` is returned through a helper or ternary expression, and `hasNext()` is dressed up with extra control flow.
## Affected test cases
### `PosCase4_Var2.java`
The class still returns itself as the iterator and still lacks a real reuse guard. The helper only hides that.
```java
// A class implements Iterable, declares an iterator() method that returns "this", and any declared method named "hasNext" has a body with exactly one statement, which is not a return statement returning the boolean literal false should be flagged as lacking a guard against multiple concurrent iterations.
package scensct.var.pos;
import java.util.Iterator;
public class PosCase4_Var2 implements Iterable, Iterator {
// Keep iterator returning this
public Iterator iterator() {
Iterator it = this;
return it;
}
// hasNext with a try-catch that doesn't affect the single return statement
public boolean hasNext() {
try {
return 1 < 2;
} catch (Exception e) {
throw new RuntimeException(e);
}
}
// next with a dummy operation
public Object next() {
System.gc();
return null;
}
}
```
### `PosCase4_Var3.java`
This is the same unsafe self-iterable pattern with one extra layer of indirection.
```java
// A class implements Iterable, declares an iterator() method that returns "this", and any declared method named "hasNext" has a body with exactly one statement, which is not a return statement returning the boolean literal false should be flagged as lacking a guard against multiple concurrent iterations.
package scensct.var.pos;
import java.util.Iterator;
public class PosCase4_Var3 implements Iterable, Iterator {
// Inline a helper method call in iterator
public Iterator iterator() {
return getSelf();
}
private Iterator getSelf() {
return this;
}
// hasNext with a single statement that computes true via method call
public boolean hasNext() {
return checkHasNext();
}
private boolean checkHasNext() {
return true;
}
public Object next() {
return "dummy";
}
}
```
### `PosCase4_Var4.java`
The control-flow refactoring does not change the fact that repeated iteration is still unsafe.
```java
// A class implements Iterable, declares an iterator() method that returns "this", and any declared method named "hasNext" has a body with exactly one statement, which is not a return statement returning the boolean literal false should be flagged as lacking a guard against multiple concurrent iterations.
package scensct.var.pos;
import java.util.Iterator;
public class PosCase4_Var4 implements Iterable, Iterator {
// iterator returns this via a ternary operator (trivial)
public Iterator iterator() {
return (System.currentTimeMillis() > 0) ? this : this;
}
// hasNext with a single statement that is not a simple return false
public boolean hasNext() {
for (int i = 0; i < 1; i++) {
return i == 0;
}
return false; // This line is unreachable, but the method body still has one reachable return
}
public Object next() {
return Integer.valueOf(42);
}
}
```
## Cause analysis
The query appears to rely on a very direct `return this` / `return false` style match. Once either method is wrapped in a helper, a ternary, or a small control-flow construct, it stops recognizing the same iterator misuse pattern.
That is narrower than it should be. These implementations are still self-iterating objects without a real reentrancy guard.
## References
None known.
Beitragsleitfaden
Rechercherichtung
Start with the Language Abuse/IterableIterator.ql checker and compare its matching logic with PosCase4_Var2.java, PosCase4_Var3.java, and PosCase4_Var4.java. Update the checker and regression coverage so all three self-iterating implementations are flagged despite helper calls or trivial control flow, while preserving the intended guard behavior.
Vom Indexierungsmodell aus dem Issue-Text verfasst.
Bewertung
- Tech-Stack
- java
- Bereich
- devtools, security
- Issue-Typ
- Bug
- Schwierigkeit
- 4/5
- Geschätzter Aufwand
- 3-5 Tage
- Aktivitätsstatus
- Ruhig
- Klarheit
- Klar beschrieben
- Anfängerfreundlichkeit
- 52/100