Skip to content

Commit 9916bbb

Browse files
MathiasVPMathiasVP
committed
PS: Add ZipSlip query.
1 parent 1f775b9 commit 9916bbb

7 files changed

Lines changed: 250 additions & 0 deletions

File tree

‎powershell/ql/lib/semmle/code/powershell/security/ZipSlipCustomizations.qll‎

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
/**
2+
* Provides default sources, sinks and sanitizers for reasoning about
3+
* zip slip vulnerabilities, as well as extension points for
4+
* adding your own.
5+
*/
6+
7+
private import semmle.code.powershell.dataflow.DataFlow
8+
import semmle.code.powershell.ApiGraphs
9+
private import semmle.code.powershell.dataflow.flowsources.FlowSources
10+
private import semmle.code.powershell.Cfg
11+
12+
module ZipSlip {
13+
/**
14+
* A data flow source for zip slip vulnerabilities.
15+
*/
16+
abstract class Source extends DataFlow::Node {
17+
/** Gets a string that describes the type of this flow source. */
18+
abstract string getSourceType();
19+
}
20+
21+
/**
22+
* A data flow sink for zip slip vulnerabilities.
23+
*/
24+
abstract class Sink extends DataFlow::Node {
25+
abstract string getSinkType();
26+
}
27+
28+
/**
29+
* A sanitizer for zip slip vulnerabilities.
30+
*/
31+
abstract class Sanitizer extends DataFlow::Node { }
32+
33+
/**
34+
* Access to the `FullName` property of the archive item
35+
*/
36+
class ArchiveEntryFullName extends Source {
37+
ArchiveEntryFullName() {
38+
this =
39+
API::getTopLevelMember("system")
40+
.getMember("io")
41+
.getMember("compression")
42+
.getMember("zipfile")
43+
.getReturn("openread")
44+
.getMember("entries")
45+
.getAnElement()
46+
.getField("fullname")
47+
.asSource()
48+
}
49+
50+
override string getSourceType() {
51+
result = "read of System.IO.Compression.ZipArchiveEntry.FullName"
52+
}
53+
}
54+
55+
/**
56+
* Argument to extract to file extension method
57+
*/
58+
class SinkCompressionExtractToFileArgument extends Sink {
59+
SinkCompressionExtractToFileArgument() {
60+
exists(DataFlow::CallNode call |
61+
call =
62+
API::getTopLevelMember("system")
63+
.getMember("io")
64+
.getMember("compression")
65+
.getMember("zipfileextensions")
66+
.getMember("extracttofile")
67+
.asCall() and
68+
this = call.getArgument(1)
69+
)
70+
}
71+
72+
override string getSinkType() { result = "argument to file extraction" }
73+
}
74+
75+
class SinkFileOpenArgument extends Sink {
76+
SinkFileOpenArgument() {
77+
exists(DataFlow::CallNode call |
78+
call =
79+
API::getTopLevelMember("system")
80+
.getMember("io")
81+
.getMember("file")
82+
.getMethod(["open", "openwrite", "create"])
83+
.asCall() and
84+
this = call.getArgument(0)
85+
)
86+
}
87+
88+
override string getSinkType() { result = "argument to file opening" }
89+
}
90+
91+
private class ExternalZipSlipSink extends Sink {
92+
ExternalZipSlipSink() { this = ModelOutput::getASinkNode("zip-slip").asSink() }
93+
94+
override string getSinkType() { result = "zip slip" }
95+
}
96+
}

‎powershell/ql/lib/semmle/code/powershell/security/ZipSlipQuery.qll‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
/**
2+
* Provides a taint tracking configuration for reasoning about
3+
* zip slip (CWE-022).
4+
*
5+
* Note, for performance reasons: only import this file if
6+
* `ZipSlipFlow` is needed, otherwise
7+
* `ZipSlipCustomizations` should be imported instead.
8+
*/
9+
10+
import powershell
11+
import semmle.code.powershell.dataflow.flowsources.FlowSources
12+
import semmle.code.powershell.dataflow.DataFlow
13+
import semmle.code.powershell.dataflow.TaintTracking
14+
import ZipSlipCustomizations::ZipSlip
15+
16+
module Config implements DataFlow::ConfigSig {
17+
predicate isSource(DataFlow::Node source) { source instanceof Source }
18+
19+
predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
20+
21+
predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer }
22+
}
23+
24+
module ZipSlipFlow = TaintTracking::Global<Config>;

‎powershell/ql/src/queries/security/cwe-022/ZipSlip.qhelp‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
<overview>
6+
<p>Extracting files from a malicious zip file, or similar type of archive,
7+
is at risk of directory traversal attacks if filenames from the archive are
8+
not properly validated.</p>
9+
10+
<p>Zip archives contain archive entries representing each file in the archive. These entries
11+
include a file path for the entry, but these file paths are not restricted and may contain
12+
unexpected special elements such as the directory traversal element (<code>..</code>). If these
13+
file paths are used to create a filesystem path, then a file operation may happen in an
14+
unexpected location. This can result in sensitive information being
15+
revealed or deleted, or an attacker being able to influence behavior by modifying unexpected
16+
files.</p>
17+
18+
<p>For example, if a zip file contains a file entry <code>..\sneaky-file</code>, and the zip file
19+
is extracted to the directory <code>c:\output</code>, then naively combining the paths would result
20+
in an output file path of <code>c:\output\..\sneaky-file</code>, which would cause the file to be
21+
written to <code>c:\sneaky-file</code>.</p>
22+
23+
</overview>
24+
<recommendation>
25+
26+
<p>Ensure that output paths constructed from zip archive entries are validated to prevent writing
27+
files to unexpected locations.</p>
28+
29+
<p>The recommended way of writing an output file from a zip archive entry is to conduct the following in sequence:</p>
30+
31+
<ol>
32+
<li>Use <code>Path.Combine(destinationDirectory, archiveEntry.FullName)</code> to determine the raw
33+
output path.</li>
34+
<li>Use <code>Path.GetFullPath(..)</code> on the raw output path to resolve any directory traversal
35+
elements.</li>
36+
<li>Use <code>Path.GetFullPath(destinationDirectory + Path.DirectorySeparatorChar)</code> to
37+
determine the fully resolved path of the destination directory.</li>
38+
<li>Validate that the resolved output path <code>StartsWith</code> the resolved destination
39+
directory, aborting if this is not true.</li>
40+
</ol>
41+
42+
<p>Another alternative is to validate archive entries against a whitelist of expected files.</p>
43+
44+
</recommendation>
45+
<example>
46+
47+
<p>In this example, a file path taken from a zip archive item entry is combined with a
48+
destination directory. The result is used as the destination file path without verifying that
49+
the result is within the destination directory. If provided with a zip file containing an archive
50+
path like <code>..\sneaky-file</code>, then this file would be written outside the destination
51+
directory.</p>
52+
53+
<sample src="examples/ZipSlipBad.ps1" />
54+
55+
<p>To fix this vulnerability, we can instead use the PowerShell command <code>Expand-Archive</code>
56+
which is safe against this vulnerability by default starting from PowerShell 5.0.</p>
57+
58+
<sample src="examples/ZipSlipGood1.ps1" />
59+
60+
<p>If you need to use the lower-level functionality offered by <code>System.IO.Compression.ZipFile</code>
61+
we need to make three changes. Firstly, we need to resolve any directory traversal or other special
62+
characters in the path by using <code>Path.GetFullPath</code>. Secondly, we need to identify the
63+
destination output directory, again using <code>Path.GetFullPath</code>, this time on the output directory.
64+
Finally, we need to ensure that the resolved output starts with the resolved destination directory, and
65+
throw an exception if this is not the case.</p>
66+
67+
<sample src="examples/ZipSlipGood2.ps1" />
68+
69+
</example>
70+
<references>
71+
72+
<li>
73+
Snyk:
74+
<a href="https://snyk.io/research/zip-slip-vulnerability">Zip Slip Vulnerability</a>.
75+
</li>
76+
<li>
77+
OWASP:
78+
<a href="https://owasp.org/www-community/attacks/Path_Traversal">Path Traversal</a>.
79+
</li>
80+
81+
</references>
82+
</qhelp>

‎powershell/ql/src/queries/security/cwe-022/ZipSlip.ql‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
/**
2+
* @name Arbitrary file access during archive extraction ("Zip Slip")
3+
* @description Extracting files from a malicious ZIP file, or similar type of archive, without
4+
* validating that the destination file path is within the destination directory
5+
* can allow an attacker to unexpectedly gain access to resources.
6+
* @kind path-problem
7+
* @id ps/zipslip
8+
* @problem.severity error
9+
* @security-severity 7.5
10+
* @precision high
11+
* @tags security
12+
* external/cwe/cwe-022
13+
*/
14+
15+
import powershell
16+
import semmle.code.powershell.security.ZipSlipQuery
17+
import ZipSlipFlow::PathGraph
18+
19+
from ZipSlipFlow::PathNode source, ZipSlipFlow::PathNode sink
20+
where ZipSlipFlow::flowPath(source, sink)
21+
select source.getNode(), source, sink,
22+
"Unsanitized archive entry, which may contain '..', is used in a $@.", sink.getNode(),
23+
"file system operation"

‎powershell/ql/src/queries/security/cwe-022/examples/ZipSlipBad.ps1‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
$zip = [System.IO.Compression.ZipFile]::OpenRead("MyPath\to\archive.zip")
2+
3+
foreach ($entry in $zip.Entries) {
4+
$targetPath = Join-Path $extractPath $entry.FullName
5+
6+
# BAD: No validation of $targetPath
7+
[System.IO.Compression.ZipFileExtensions]::ExtractToFile($entry, $targetPath)
8+
}
9+
$zip.Dispose()

‎powershell/ql/src/queries/security/cwe-022/examples/ZipSlipGood1.ps1‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Expand-Archive -Path "MyPath\to\archive.zip" -DestinationPath $extractPath -Force

‎powershell/ql/src/queries/security/cwe-022/examples/ZipSlipGood2.ps1‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
$zip = [System.IO.Compression.ZipFile]::OpenRead("MyPath\to\archive.zip")
2+
3+
foreach ($entry in $zip.Entries) {
4+
$targetPath = Join-Path $extractPath $entry.FullName
5+
$fullTargetPath = [System.IO.Path]::GetFullPath($targetPath)
6+
7+
# GOOD: Validate that the full path is within the intended extraction directory
8+
$extractRoot = [System.IO.Path]::GetFullPath($extractPath)
9+
if ($fullTargetPath.StartsWith($extractRoot)) {
10+
[System.IO.Compression.ZipFileExtensions]::ExtractToFile($entry, $fullTargetPath, $true)
11+
} else {
12+
Write-Warning "Skipping potentially malicious entry: $($entry.FullName)"
13+
}
14+
}
15+
$zip.Dispose()

0 commit comments

Comments
 (0)