github / github/codeql

Possible false positive "Uncontrolled data used in path expression" still after stripping path from input.

Aperta
#18,225 0 commenti 0 reazioni 0 assegnatari Vedi su GitHub
false-positive
Lingua principale
CodeQL
Stelle
10.1k
Fork
2.1k
Merge medio
2g 15h
PR unite (30g)
141

Descrizione

The .NET `System.IO.Path.GetFileName` will take a path, relative or otherwise, and return only the filename part. I am using this to sanitize the input to a Download action that serves files from a hardcoded folder. The initial code looked like this, which is in fact vulnerable:

```
[HttpGet]
public IActionResult Download(string fileName)
{
string filesDir = "Files\\FilesToServe";
string filePath = Path.Combine(filesDir, fileName);

if (System.IO.File.Exists(filePath))
{
FileStream fs = new FileStream(filePath, FileMode.Open);

return File(fs, "text/plain", fileName);
}

return NotFound();
}
```

Corrective action was taken so that only a file name with no additional path will be used to look for the file:

```
[HttpGet]
public IActionResult Download(string fileName)
{
string strippedFileName = Path.GetFileName(fileName);
string filesDir = "Files\\FilesToServe";
string filePath = Path.Combine(filesDir, strippedFileName);

if (System.IO.File.Exists(filePath))
{
FileStream fs = new FileStream(filePath, FileMode.Open);

return File(fs, "text/plain", strippedFileName);
}

return NotFound();
}
```

This still detects "Uncontrolled data used in path expression (cs/path-injection)" for `filePath` even though it has been tested to no longer serve files outside of `filesDir` when passing in parameters such as "../../../FileThatShouldntBeServed.txt".

Guida per i contributori

Apri la guida per i contributori

Direzione di ricerca

Start by reproducing the C# examples in a CodeQL test case and inspect how the cs/path-injection query models Path.GetFileName. Compare results for the original vulnerable code and the stripped-file-name version; done means the sanitized example is not flagged while the original remains detected.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Valutazione

Stack tecnologico
csharp
Ambito
security
Tipo di issue
Bug
Difficoltà
4/5
Tempo stimato
3-5 giorni
Stato di attività
Ferma
Chiarezza
Abbastanza chiara
Idoneità per principianti
45/100

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.