github / github/codeql

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

Đang mở
#21,551 1 bình luận 0 reaction 0 người được giao Xem trên GitHub
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

Mở 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

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.