github / github/codeql

False Negative: CloseReader.ql misses unclosed streams once construction moves into helpers or alternate APIs.

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

Description

Version
codeql 2.24.3

## Checker
- Checker id: `Likely Bugs/Resource Leaks/CloseReader.ql`
- Checker description: This checker detects instances of Reader, InputStream, or ZipFile objects that are created but not guaranteed to be closed on method exit, potentially causing resource leaks.

## Description of the false negative
All four samples still leave a reader or stream resource unclosed. The differences are superficial: one allocation is hidden behind a helper, one uses `Files.newInputStream(...)` instead of `new FileInputStream(...)`, one leaks a custom `InputStream`, and one leaks a `BufferedReader` built around a parameter stream.

None of those variations change the resource-management obligation.

## Affected test cases
### `PosCase1_Var4.java`
The stream is created in `openStream()` and immediately discarded by the caller. That is still a leak.

### `PosCase1_Var5.java`
`Files.newInputStream(...)` returns an `InputStream` that still needs to be closed.

### `PosCase2.java`
The custom stream object is allocated, used, and never closed.

### `PosCase4.java`
The `BufferedReader` wraps a parameter stream and is never closed, so the wrapper resource itself leaks.

## Reproduction code
### `PosCase1_Var4.java`
```java
// FileInputStream created but not closed should be flagged as resource leak.
package scensct.var.pos;

import java.io.FileInputStream;
import java.io.IOException;

public class PosCase1_Var4 {
// Variant 4: Extract creation to a helper method
private FileInputStream openStream() throws IOException {
return new FileInputStream("test.txt");
}

public void readFile() throws IOException {
openStream(); // Returned stream is not assigned or closed
}
}
```

### `PosCase1_Var5.java`
```java
// FileInputStream created but not closed should be flagged as resource leak.
package scensct.var.pos;

import java.io.FileInputStream;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Paths;

public class PosCase1_Var5 {
// Variant 5: Use Files.newInputStream (still an InputStream)
public void readFile() throws IOException {
Files.newInputStream(Paths.get("test.txt"));
// Not closed
}
}
```

### `PosCase2.java`
```java
// Custom InputStream without close() method created but not closed should be flagged.
package scensct.core.pos;

import java.io.InputStream;
import java.io.IOException;

public class PosCase2 {
// Custom InputStream that does not declare close()
static class CustomStream extends InputStream {
@Override
public int read() {
return -1;
}
// No close() method overridden
}

public void useStream() {
InputStream stream = new CustomStream(); // Instantiation with assignment
try {
stream.read(); // Use the stream to emphasize it's a resource
} catch (IOException e) {
// Ignore for test purposes
}
// Not closed
}
}
```

### `PosCase4.java`
```java
// Wrapper BufferedReader around parameter InputStream not closed should be flagged.
package scensct.core.pos;

import java.io.BufferedReader;
import java.io.InputStream;
import java.io.InputStreamReader;

public class PosCase4 {
public void wrapParameter(InputStream paramStream) {
BufferedReader reader = new BufferedReader(new InputStreamReader(paramStream)); // Wrapper assigned but not closed
}
}
```

## Cause analysis
The misses point to incomplete source modeling in `Likely Bugs/Resource Leaks/CloseReader.ql`. The rule seems strongest on very direct allocation forms, but it loses coverage once the resource comes back from a helper or from another factory API.

That is a problem for a leak checker. Real code uses utility methods and alternative constructors all the time, and the need to close the returned resource does not disappear when the allocation becomes one step less direct.

Contributor guide

Open the contributing guide

Research direction

Start with Likely Bugs/Resource Leaks/CloseReader.ql and compare its source modeling with the four examples: PosCase1_Var4.java, PosCase1_Var5.java, PosCase2.java, and PosCase4.java. Run the affected query tests and confirm that each unclosed helper result, factory-created stream, custom InputStream, and BufferedReader wrapper is reported without regressing existing cases.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
security
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.