github / github/codeql

False Negative: CloseWriter.ql misses leaked wrapped streams when the close is skipped by an early return or hidden in a factory.

Ouverte
#21,534 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 18 h
PR mergées (30 j)
152

Description

Version
codeql 2.24.3

## Checker
- Checker id: `Likely Bugs/Resource Leaks/CloseWriter.ql`
- Checker description: This checker detects instances where a Writer or OutputStream is created but not guaranteed to be closed on method exit, potentially causing resource leaks.

## Description of the false negative
Both examples still leak the underlying file stream. One exits the method before the wrapper is closed. The other moves stream construction into a helper and then never closes the returned wrapper.

Those are ordinary resource-leak scenarios. Neither refactoring changes the ownership or lifetime of the stream.

## Affected test cases
### `PosCase2_Var3.java`
The `return` bypasses the only `close()` call. `bos` and the wrapped `FileOutputStream` both remain unclosed on the live path.

```java
// FileOutputStream passed to BufferedOutputStream where inner resource not guaranteed to be closed should be flagged.
package scensct.var.pos;

import java.io.BufferedOutputStream;
import java.io.FileOutputStream;
import java.io.IOException;

public class PosCase2_Var3 {
public void writeBuffered() throws IOException {
// Variant 3: Introduce early return that skips close
FileOutputStream fos = new FileOutputStream("data.bin");
BufferedOutputStream bos = new BufferedOutputStream(fos);
bos.write(1);
if (System.currentTimeMillis() > 0) {
return; // exit without closing
}
// Unreachable code, close never called
bos.close();
}
}
```

### `PosCase2_Var5.java`
The factory method only hides allocation. It does not transfer cleanup responsibility anywhere else, and the caller still never closes the returned stream.

```java
// FileOutputStream passed to BufferedOutputStream where inner resource not guaranteed to be closed should be flagged.
package scensct.var.pos;

import java.io.BufferedOutputStream;
import java.io.FileOutputStream;
import java.io.IOException;

public class PosCase2_Var5 {
public void writeBuffered() throws IOException {
// Variant 5: Wrap in a method that returns the BufferedOutputStream without closing
BufferedOutputStream bos = openBufferedStream("data.bin");
bos.write(1);
// Not closed
}

private BufferedOutputStream openBufferedStream(String file) throws IOException {
FileOutputStream fos = new FileOutputStream(file);
return new BufferedOutputStream(fos);
}
}
```

## Cause analysis
`Likely Bugs/Resource Leaks/CloseWriter.ql` looks too dependent on one direct construction-and-close pattern. As soon as the leak is expressed through an early return or a helper that returns the wrapper, the result disappears.

That is too brittle for a resource-leak query. In real code, stream creation is often factored into small helpers, and missing close calls frequently happen on short-circuit exits.

Guide de contribution

Ouvrir le guide de contribution

Piste de recherche

Commencez par lire Likely Bugs/Resource Leaks/CloseWriter.ql et examinez les cas concernés dans PosCase2_Var3.java et PosCase2_Var5.java. Suivez la manière dont la requête gère les retours anticipés et les streams créés par des helpers, puis exécutez les tests CloseWriter concernés. Le travail est terminé lorsque les deux cas sont signalés comme des leaks sans dégrader la couverture existante.

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

Évaluation

Stack technique
java
Domaine
security
Type d'issue
Bug
Difficulté
4/5
Temps estimé
3-5 jours
Activité
Calme
Clarté
Clairement spécifiée
Accessibilité débutants
52/100

Recevez les nouvelles issues par e-mail

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