Skip to content

Conversation

@MathiasVP
Copy link
Collaborator

@MathiasVP MathiasVP commented Sep 23, 2025

This PR adds another classic query: finding zip slip vulnerabilities.

The query works in the sense that it does find what its supposed to find. i.e., it detects:

$zip = [System.IO.Compression.ZipFile]::OpenRead("MyPath\to\archive.zip")
foreach ($entry in $zip.Entries) {
    $targetPath = Join-Path $extractPath $entry.FullName
    [System.IO.Compression.ZipFileExtensions]::ExtractToFile($entry, $targetPath) # BAD
}

However, because we haven't added support for proper barriers guards to PowerShell we currently also flags this false positive:

foreach ($entry in $zip.Entries) {
    $targetPath = Join-Path $extractPath $entry.FullName
    $fullTargetPath = [System.IO.Path]::GetFullPath($targetPath)

    $extractRoot = [System.IO.Path]::GetFullPath($extractPath)
    if ($fullTargetPath.StartsWith($extractRoot)) {
        [System.IO.Compression.ZipFileExtensions]::ExtractToFile($entry, $fullTargetPath) # GOOD [FALSE POSITIVE]
    }
}

I'll use this motivation to start working on proper barrier guard support for PowerShell soon. That should be relatively easy now that GitHub has added a shared guards library.

Commit-by-commit review recommended. It took quite a lot of small improvement in various internal libraries to get this working. In particular:

  • 1f775b9 adds another user-facing API to API graphs so that we can mark zip.Entities as a flow source.
  • 9916bbb adds the actual query.
  • 8d3f6b8 adds query tests. At this point they all fail, but we'll fix that in the subsequence commits.
  • ca2cf5e adds some much-needed flow summaries needed to get the tests working. 869c613 also adds separate tests for these summaries.

I then discovered a bug in the control-flow class for foreach: the predicate for getting the "iterable" part of a foreach (i.e, xs in foreach(x in xs) { ... }) didn't work because it has some special handling in the control-flow graph. So 831f25d adds a test for this and 2ec78ed fixes this.

I then discovered another bug around foreach loops: we didn't have a read step from xs to x in:

foreach(x in xs) {
 ...
}

So 4276405 adds a test for this and 5692eb0 fixes this.

And finally the query now detects the required cases as demonstrated by 93a3833 🎉

Along the way I also did a drive-by improvement to DataFlow::ObjectCreationNode in ca20eb5. This was needed for an earlier version of the query, but ended up not being necessary in the end. It is still a valuable change, though.

@MathiasVP MathiasVP merged commit a130224 into main Sep 24, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants