github / github/codeql

False Positive: CloseReader.ql reports resources that are either transferred to another owner or closed by a wrapper.

Ouverte
#21,530 1 commentaire 0 réactions 0 personnes assignées Voir sur GitHub
question
Langage dominant
CodeQL
Étoiles
10.1k
Forks
2.1k
Merge moyen
2 j 15 h
PR mergées (30 j)
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 positive
Neither sample is a straightforward leak.

In `NegCase3.java`, the `FileInputStream` is immediately wrapped by a type whose `close()` implementation delegates to the underlying stream. In `NegCase7.java`, the stream is passed into another object and stored there, which is an ownership-transfer pattern rather than a local leak.

## Affected test cases
### `NegCase3.java`
The wrapper is the owner. Reporting the inner `FileInputStream` as leaked means the query is not trusting a wrapper that clearly forwards `close()`.

### `NegCase7.java`
The stream escapes through `field = new Outer(is)`. That may or may not be a great API design, but it is not the same as dropping a local resource on the floor.

## Reproduction code
### `NegCase3.java`
```java
// A FileInputStream wrapped in a custom closeable with empty close should not be flagged as a leak.
package scensct.core.neg;

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

public class NegCase3 {
// Custom wrapper with empty close method.
static class NoCloseWrapper extends InputStream {
private final InputStream inner;
NoCloseWrapper(InputStream in) { this.inner = in; }
@Override public int read() throws IOException { return inner.read(); }
@Override public void close() throws IOException { inner.close(); } // Now delegates closing.
}

public void test() throws IOException {
// Scenario 3: Transitive closeable parent satisfies noNeedToClose.
new NoCloseWrapper(new FileInputStream("test.txt")); // Inner resource managed by wrapper. // [REPORTED LINE]
}
}
```

### `NegCase7.java`
```java
// A FileInputStream passed to a locally initialized outer constructor and escaping should not be flagged as a leak.
package scensct.core.neg;

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

public class NegCase7 {
static class Outer {
private final InputStream inner;
Outer(InputStream in) { this.inner = in; } // No exceptions declared.
}

private Outer field;

public void test() throws IOException {
// Scenario 7: Resource not assigned, passed to constructor, escapes via field.
InputStream is = new FileInputStream("test.txt"); // [REPORTED LINE]
field = new Outer(is); // Resource escapes, no leak.
}
}
```

## Cause analysis
These results suggest two over-approximations in `Likely Bugs/Resource Leaks/CloseReader.ql`.

First, the query is not reliably recognizing wrapper classes that take responsibility for the underlying resource. Second, it is treating ownership transfer as if it were equivalent to local abandonment. Both behaviors inflate the result set with cases that developers will not read as direct leaks in the current method.

Guide de contribution

Ouvrir le guide de contribution

Piste de recherche

Lisez Likely Bugs/Resource Leaks/CloseReader.ql et comparez son comportement avec NegCase3.java et NegCase7.java, puis exécutez les tests de requête concernés. C’est terminé lorsque ces deux lignes signalées ne sont plus marquées, tandis que les cas réels de Reader, InputStream et ZipFile non fermés restent détectés.

Rédigé par le modèle d'indexation à partir du texte de l'issue.

Évaluation

Stack technique
java
Domaine
devtools, security
Type d'issue
Bug
Difficulté
4/5
Temps estimé
3-5 jours
Activité
Calme
Clarté
Plutôt claire
Accessibilité débutants
50/100

Recevez les nouvelles issues par e-mail

Un résumé court des issues GitHub adaptées aux débutants.