github / github/codeql

False Negative: IterableIterator.ql misses `iterator() == this` implementations once the guard logic is hidden behind helpers or trivial control flow.

Abierto
#21,551 1 comentario 0 reacciones 0 asignados Ver en GitHub
Lenguaje dominante
CodeQL
Estrellas
10.1k
Forks
2.1k
Merge medio
2 d 15 h
PR fusionados (30 d)
141

Descripción

# 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.

Guía de contribución

Abrir la guía de contribución

Línea de trabajo

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.

Escrito por el modelo de indexación a partir del texto del issue.

Evaluación

Stack tecnológico
java
Área
devtools, security
Tipo de issue
Error
Dificultad
4/5
Tiempo estimado
3-5 días
Estado de actividad
Tranquilo
Claridad
Bien especificado
Aptitud para principiantes
52/100

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.