forked from github/codeql
-
Notifications
You must be signed in to change notification settings - Fork 20
PS: Add ZipSlip query #287
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 1 commit
Commits
Show all changes
15 commits
Select commit
Hold shift + click to select a range
1f775b9
PS: Add 'fieldEdge' to ApiGraphs.
MathiasVP 9916bbb
PS: Add ZipSlip query.
MathiasVP 8d3f6b8
PS: Add tests.
MathiasVP 869c613
PS: Add tests for the new summary models.
MathiasVP ca2cf5e
PS: Add some more summary models.
MathiasVP 831f25d
PS: Add test for forEach.
MathiasVP 2ec78ed
PS: Fix 'getIterableExpr' on 'ForEachStmtCfgNode'.
MathiasVP 4276405
PS: Add a testcase for the new read step.
MathiasVP e39d9f9
PS: Add some comments to 'readStep'.
MathiasVP 5692eb0
PS: Add read step for 'foreach' statements.
MathiasVP 93a3833
PS: Accept query test changes.
MathiasVP ca20eb5
PS: Drive-by improvement: Make ObjectCreationNode a CallNode.
MathiasVP 54226ec
PS: Accept test change to internal MAD ids.
MathiasVP d7a4063
PS: Respond to review comments.
MathiasVP abf320b
Merge branch 'main' into add-zipslip-query-ps
MathiasVP File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
96 changes: 96 additions & 0 deletions
96
powershell/ql/lib/semmle/code/powershell/security/ZipSlipCustomizations.qll
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,96 @@ | ||
| /** | ||
| * Provides default sources, sinks and sanitizers for reasoning about | ||
| * zip slip vulnerabilities, as well as extension points for | ||
| * adding your own. | ||
| */ | ||
|
|
||
| private import semmle.code.powershell.dataflow.DataFlow | ||
| import semmle.code.powershell.ApiGraphs | ||
| private import semmle.code.powershell.dataflow.flowsources.FlowSources | ||
| private import semmle.code.powershell.Cfg | ||
|
|
||
| module ZipSlip { | ||
| /** | ||
| * A data flow source for zip slip vulnerabilities. | ||
| */ | ||
| abstract class Source extends DataFlow::Node { | ||
| /** Gets a string that describes the type of this flow source. */ | ||
| abstract string getSourceType(); | ||
| } | ||
|
|
||
| /** | ||
| * A data flow sink for zip slip vulnerabilities. | ||
| */ | ||
| abstract class Sink extends DataFlow::Node { | ||
| abstract string getSinkType(); | ||
| } | ||
|
|
||
| /** | ||
| * A sanitizer for zip slip vulnerabilities. | ||
| */ | ||
| abstract class Sanitizer extends DataFlow::Node { } | ||
|
|
||
| /** | ||
| * Access to the `FullName` property of the archive item | ||
| */ | ||
| class ArchiveEntryFullName extends Source { | ||
| ArchiveEntryFullName() { | ||
| this = | ||
| API::getTopLevelMember("system") | ||
| .getMember("io") | ||
| .getMember("compression") | ||
| .getMember("zipfile") | ||
| .getReturn("openread") | ||
| .getMember("entries") | ||
| .getAnElement() | ||
| .getField("fullname") | ||
| .asSource() | ||
| } | ||
|
|
||
| override string getSourceType() { | ||
| result = "read of System.IO.Compression.ZipArchiveEntry.FullName" | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Argument to extract to file extension method | ||
| */ | ||
| class SinkCompressionExtractToFileArgument extends Sink { | ||
| SinkCompressionExtractToFileArgument() { | ||
| exists(DataFlow::CallNode call | | ||
| call = | ||
| API::getTopLevelMember("system") | ||
| .getMember("io") | ||
| .getMember("compression") | ||
| .getMember("zipfileextensions") | ||
| .getMember("extracttofile") | ||
| .asCall() and | ||
| this = call.getArgument(1) | ||
| ) | ||
| } | ||
|
|
||
| override string getSinkType() { result = "argument to file extraction" } | ||
| } | ||
|
|
||
| class SinkFileOpenArgument extends Sink { | ||
| SinkFileOpenArgument() { | ||
| exists(DataFlow::CallNode call | | ||
| call = | ||
| API::getTopLevelMember("system") | ||
| .getMember("io") | ||
| .getMember("file") | ||
| .getMethod(["open", "openwrite", "create"]) | ||
| .asCall() and | ||
| this = call.getArgument(0) | ||
| ) | ||
| } | ||
|
|
||
| override string getSinkType() { result = "argument to file opening" } | ||
| } | ||
|
|
||
| private class ExternalZipSlipSink extends Sink { | ||
| ExternalZipSlipSink() { this = ModelOutput::getASinkNode("zip-slip").asSink() } | ||
|
|
||
| override string getSinkType() { result = "zip slip" } | ||
| } | ||
| } | ||
24 changes: 24 additions & 0 deletions
24
powershell/ql/lib/semmle/code/powershell/security/ZipSlipQuery.qll
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| /** | ||
| * Provides a taint tracking configuration for reasoning about | ||
| * zip slip (CWE-022). | ||
| * | ||
| * Note, for performance reasons: only import this file if | ||
| * `ZipSlipFlow` is needed, otherwise | ||
| * `ZipSlipCustomizations` should be imported instead. | ||
| */ | ||
|
|
||
| import powershell | ||
| import semmle.code.powershell.dataflow.flowsources.FlowSources | ||
| import semmle.code.powershell.dataflow.DataFlow | ||
| import semmle.code.powershell.dataflow.TaintTracking | ||
| import ZipSlipCustomizations::ZipSlip | ||
|
|
||
| module Config implements DataFlow::ConfigSig { | ||
| predicate isSource(DataFlow::Node source) { source instanceof Source } | ||
|
|
||
| predicate isSink(DataFlow::Node sink) { sink instanceof Sink } | ||
|
|
||
| predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer } | ||
| } | ||
|
|
||
| module ZipSlipFlow = TaintTracking::Global<Config>; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,82 @@ | ||
| <!DOCTYPE qhelp PUBLIC | ||
| "-//Semmle//qhelp//EN" | ||
| "qhelp.dtd"> | ||
| <qhelp> | ||
| <overview> | ||
| <p>Extracting files from a malicious zip file, or similar type of archive, | ||
| is at risk of directory traversal attacks if filenames from the archive are | ||
| not properly validated.</p> | ||
|
|
||
| <p>Zip archives contain archive entries representing each file in the archive. These entries | ||
| include a file path for the entry, but these file paths are not restricted and may contain | ||
| unexpected special elements such as the directory traversal element (<code>..</code>). If these | ||
| file paths are used to create a filesystem path, then a file operation may happen in an | ||
| unexpected location. This can result in sensitive information being | ||
| revealed or deleted, or an attacker being able to influence behavior by modifying unexpected | ||
| files.</p> | ||
|
|
||
| <p>For example, if a zip file contains a file entry <code>..\sneaky-file</code>, and the zip file | ||
| is extracted to the directory <code>c:\output</code>, then naively combining the paths would result | ||
| in an output file path of <code>c:\output\..\sneaky-file</code>, which would cause the file to be | ||
| written to <code>c:\sneaky-file</code>.</p> | ||
|
|
||
| </overview> | ||
| <recommendation> | ||
|
|
||
| <p>Ensure that output paths constructed from zip archive entries are validated to prevent writing | ||
| files to unexpected locations.</p> | ||
|
|
||
| <p>The recommended way of writing an output file from a zip archive entry is to conduct the following in sequence:</p> | ||
|
|
||
| <ol> | ||
| <li>Use <code>Path.Combine(destinationDirectory, archiveEntry.FullName)</code> to determine the raw | ||
| output path.</li> | ||
| <li>Use <code>Path.GetFullPath(..)</code> on the raw output path to resolve any directory traversal | ||
| elements.</li> | ||
| <li>Use <code>Path.GetFullPath(destinationDirectory + Path.DirectorySeparatorChar)</code> to | ||
| determine the fully resolved path of the destination directory.</li> | ||
| <li>Validate that the resolved output path <code>StartsWith</code> the resolved destination | ||
| directory, aborting if this is not true.</li> | ||
| </ol> | ||
|
|
||
| <p>Another alternative is to validate archive entries against a whitelist of expected files.</p> | ||
|
|
||
| </recommendation> | ||
| <example> | ||
|
|
||
| <p>In this example, a file path taken from a zip archive item entry is combined with a | ||
| destination directory. The result is used as the destination file path without verifying that | ||
| the result is within the destination directory. If provided with a zip file containing an archive | ||
| path like <code>..\sneaky-file</code>, then this file would be written outside the destination | ||
| directory.</p> | ||
|
|
||
| <sample src="examples/ZipSlipBad.ps1" /> | ||
|
|
||
| <p>To fix this vulnerability, we can instead use the PowerShell command <code>Expand-Archive</code> | ||
| which is safe against this vulnerability by default starting from PowerShell 5.0.</p> | ||
|
|
||
| <sample src="examples/ZipSlipGood1.ps1" /> | ||
|
|
||
| <p>If you need to use the lower-level functionality offered by <code>System.IO.Compression.ZipFile</code> | ||
| we need to make three changes. Firstly, we need to resolve any directory traversal or other special | ||
| characters in the path by using <code>Path.GetFullPath</code>. Secondly, we need to identify the | ||
| destination output directory, again using <code>Path.GetFullPath</code>, this time on the output directory. | ||
| Finally, we need to ensure that the resolved output starts with the resolved destination directory, and | ||
| throw an exception if this is not the case.</p> | ||
|
|
||
| <sample src="examples/ZipSlipGood2.ps1" /> | ||
|
|
||
| </example> | ||
| <references> | ||
|
|
||
| <li> | ||
| Snyk: | ||
| <a href="https://snyk.io/research/zip-slip-vulnerability">Zip Slip Vulnerability</a>. | ||
| </li> | ||
| <li> | ||
| OWASP: | ||
| <a href="https://owasp.org/www-community/attacks/Path_Traversal">Path Traversal</a>. | ||
| </li> | ||
|
|
||
| </references> | ||
| </qhelp> |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| /** | ||
| * @name Arbitrary file access during archive extraction ("Zip Slip") | ||
| * @description Extracting files from a malicious ZIP file, or similar type of archive, without | ||
| * validating that the destination file path is within the destination directory | ||
| * can allow an attacker to unexpectedly gain access to resources. | ||
| * @kind path-problem | ||
| * @id ps/zipslip | ||
| * @problem.severity error | ||
| * @security-severity 7.5 | ||
| * @precision high | ||
| * @tags security | ||
| * external/cwe/cwe-022 | ||
| */ | ||
|
|
||
| import powershell | ||
| import semmle.code.powershell.security.ZipSlipQuery | ||
| import ZipSlipFlow::PathGraph | ||
|
|
||
| from ZipSlipFlow::PathNode source, ZipSlipFlow::PathNode sink | ||
| where ZipSlipFlow::flowPath(source, sink) | ||
| select source.getNode(), source, sink, | ||
| "Unsanitized archive entry, which may contain '..', is used in a $@.", sink.getNode(), | ||
| "file system operation" |
9 changes: 9 additions & 0 deletions
9
powershell/ql/src/queries/security/cwe-022/examples/ZipSlipBad.ps1
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| $zip = [System.IO.Compression.ZipFile]::OpenRead("MyPath\to\archive.zip") | ||
|
|
||
| foreach ($entry in $zip.Entries) { | ||
| $targetPath = Join-Path $extractPath $entry.FullName | ||
|
|
||
| # BAD: No validation of $targetPath | ||
| [System.IO.Compression.ZipFileExtensions]::ExtractToFile($entry, $targetPath) | ||
| } | ||
| $zip.Dispose() |
1 change: 1 addition & 0 deletions
1
powershell/ql/src/queries/security/cwe-022/examples/ZipSlipGood1.ps1
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| Expand-Archive -Path "MyPath\to\archive.zip" -DestinationPath $extractPath -Force |
15 changes: 15 additions & 0 deletions
15
powershell/ql/src/queries/security/cwe-022/examples/ZipSlipGood2.ps1
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| $zip = [System.IO.Compression.ZipFile]::OpenRead("MyPath\to\archive.zip") | ||
|
|
||
| foreach ($entry in $zip.Entries) { | ||
| $targetPath = Join-Path $extractPath $entry.FullName | ||
| $fullTargetPath = [System.IO.Path]::GetFullPath($targetPath) | ||
|
|
||
| # GOOD: Validate that the full path is within the intended extraction directory | ||
| $extractRoot = [System.IO.Path]::GetFullPath($extractPath) | ||
| if ($fullTargetPath.StartsWith($extractRoot)) { | ||
| [System.IO.Compression.ZipFileExtensions]::ExtractToFile($entry, $fullTargetPath, $true) | ||
| } else { | ||
| Write-Warning "Skipping potentially malicious entry: $($entry.FullName)" | ||
| } | ||
| } | ||
| $zip.Dispose() |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.