False Negative: IterableIterator.ql misses `iterator() == this` implementations once the guard logic is hidden behind helpers or trivial control flow.
- Ngôn ngữ chính
- CodeQL
- Star
- 10.1k
- Fork
- 2.1k
- Merge trung bình
- 2 ngày 15 giờ
- Pull request đã merge (30 ngày)
- 141
Mô tả
# 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.
Hướng dẫn đóng góp
Hướng nghiên cứu
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.
Do mô hình lập chỉ mục viết ra từ nội dung của issue.
Đánh giá
- Công nghệ
- java
- Lĩnh vực
- devtools, security
- Loại issue
- Lỗi
- Độ khó
- 4/5
- Thời gian dự kiến
- 3-5 ngày
- Mức độ hoạt động
- Ít trao đổi
- Độ rõ ràng
- Đặc tả rõ ràng
- Mức phù hợp với người mới
- 52/100