josh zipslip improvements

This commit is contained in:
Dilan Bhalla
2023-04-24 14:49:27 -07:00
parent 885d2491d8
commit b0e4305c95
5 changed files with 540 additions and 163 deletions

View File

@@ -3,147 +3,431 @@
*/
import csharp
private import semmle.code.csharp.controlflow.Guards
import semmle.code.csharp.dataflow.TaintTracking2
import semmle.code.csharp.dataflow.TaintTracking3
import semmle.code.csharp.controlflow.Guards
import semmle.code.csharp.dataflow.DataFlow::DataFlow::PathGraph
/**
* A data flow source for unsafe zip extraction.
*/
abstract class Source extends DataFlow::Node { }
private abstract class AbstractSanitizerMethod extends Method { }
/**
* A data flow sink for unsafe zip extraction.
*/
abstract class Sink extends DataFlow::ExprNode { }
class MethodSystemStringStartsWith extends AbstractSanitizerMethod {
MethodSystemStringStartsWith() { this.getQualifiedName() = "System.String.StartsWith" }
}
/**
* A sanitizer for unsafe zip extraction.
*/
abstract class Sanitizer extends DataFlow::ExprNode { }
private abstract class UnsanitizedPathCombiner extends Expr { }
/**
* DEPRECATED: Use `Sanitizer` instead.
*
* A guard for unsafe zip extraction.
*/
abstract deprecated class SanitizerGuard extends DataFlow::BarrierGuard { }
/** A taint tracking configuration for Zip Slip */
class TaintTrackingConfiguration extends TaintTracking::Configuration {
TaintTrackingConfiguration() { this = "ZipSlipTaintTracking" }
override predicate isSource(DataFlow::Node source) { source instanceof Source }
override predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
override predicate isSanitizer(DataFlow::Node node) { node instanceof Sanitizer }
deprecated override predicate isSanitizerGuard(DataFlow::BarrierGuard guard) {
guard instanceof SanitizerGuard
class PathCombinerViaMethodCall extends UnsanitizedPathCombiner {
PathCombinerViaMethodCall() {
this.(MethodCall).getTarget().hasQualifiedName("System.IO.Path", "Combine")
}
}
/** An access to the `FullName` property of a `ZipArchiveEntry`. */
class ArchiveFullNameSource extends Source {
ArchiveFullNameSource() {
exists(PropertyAccess pa | this.asExpr() = pa |
pa.getTarget().getDeclaringType().hasQualifiedName("System.IO.Compression", "ZipArchiveEntry") and
pa.getTarget().getName() = "FullName"
class PathCombinerViaStringInterpolation extends UnsanitizedPathCombiner {
PathCombinerViaStringInterpolation() { exists(InterpolatedStringExpr e | this = e) }
}
class PathCombinerViaStringConcatenation extends UnsanitizedPathCombiner {
PathCombinerViaStringConcatenation() { exists(AddExpr e | this = e) }
}
class MethodCallGetFullPath extends MethodCall {
MethodCallGetFullPath() { this.getTarget().hasQualifiedName("System.IO.Path", "GetFullPath") }
}
class GetFullPathToQualifierTaintTrackingConfiguration extends TaintTracking3::Configuration {
GetFullPathToQualifierTaintTrackingConfiguration() {
this = "GetFullPathToQualifierTaintTrackingConfiguration"
}
override predicate isSource(DataFlow::Node node) {
exists(MethodCallGetFullPath mcGetFullPath | node = DataFlow::exprNode(mcGetFullPath))
}
override predicate isSink(DataFlow::Node node) {
exists(MethodCall mc |
mc.getTarget() instanceof MethodSystemStringStartsWith and
node.asExpr() = mc.getQualifier()
)
}
}
/** An argument to the `ExtractToFile` extension method. */
class ExtractToFileArgSink extends Sink {
ExtractToFileArgSink() {
/**
* PathCombinerToGetFullPathTaintTrackingConfiguration - A Taint Tracking configuration that tracks
* a File path combining expression (Such as string concatenation, Path.Combine, or string interpolation),
* to a Path.GetFullPath method call's argument.
*
* We need this because we need to find a safe sequence of operations wherein
* - An absolute path is created (uncanonicalized)
* - The Path is canonicalized
*
* If the operations are in the opposite order, the resultant may still contain path traversal characters,
* as you cannot fully resolve a relative path. So we must ascertain that they are conducted in this sequence.
*/
class PathCombinerToGetFullPathTaintTrackingConfiguration extends TaintTracking3::Configuration {
PathCombinerToGetFullPathTaintTrackingConfiguration() {
this = "PathCombinerToGetFullPathTaintTrackingConfiguration "
}
/**
* We are looking for the result of some Path combining operation (String concat, Path.Combine, etc.)
*/
override predicate isSource(DataFlow::Node node) {
exists(UnsanitizedPathCombiner pathCombiner | node = DataFlow::exprNode(pathCombiner))
}
/**
* Find the first (and only) argument of Path.GetFullPath, so we make sure that our expression
* first goes through some path combining function, and then is canonicalized.
*/
override predicate isSink(DataFlow::Node node) {
exists(MethodCallGetFullPath mcGetFullPath |
node = DataFlow::exprNode(mcGetFullPath.getArgument(0))
)
}
}
/** Predicate to check for a safe sequence of events
* Path.Combine THEN Path.GetFullPath is applied (with possibly arbitrary mutations)
*/
private predicate safeCombineGetFullPathSequence(MethodCallGetFullPath mcGetFullPath, Expr q) {
exists(UnsanitizedPathCombiner source, PathCombinerToGetFullPathTaintTrackingConfiguration taintTracking |
taintTracking.hasFlow(DataFlow::exprNode(source), DataFlow::exprNode(mcGetFullPath.getArgument(0)))
) and
exists(GetFullPathToQualifierTaintTrackingConfiguration qualifierTaintTracker |
qualifierTaintTracker.hasFlow(DataFlow::exprNode(mcGetFullPath), DataFlow::exprNode(q))
)
}
/**
* The set of /valid/ Guards of RootSanitizerMethodCall.
*
* IN CONJUNCTION with BOTH
* Path.Combine
* AND Path.GetFullPath
* OR
* There is a direct flow from Path.GetFullPath to qualifier of RootSanitizerMethodCall.
*
* It is not simply enough for the qualifier of String.StartsWith
* to pass through Path.Combine without also passing through GetFullPath AFTER.
*/
class RootSanitizerMethodCall extends SanitizerMethodCall {
RootSanitizerMethodCall() {
exists(MethodSystemStringStartsWith sm | this.getTarget() = sm) and
exists(Expr q, AbstractValue v |
this.getQualifier() = q and
v.(AbstractValues::BooleanValue).getValue() = true and
exists(MethodCallGetFullPath mcGetFullPath | safeCombineGetFullPathSequence(mcGetFullPath, q))
)
}
override Expr getFilePathArgument() { result = this.getQualifier() }
}
/**
* The set of Guards of RootSanitizerMethodCall that are used IN CONJUNCTION with
* Path.GetFullPath - it is not simply enough for the qualifier of String.StartsWith
* to pass through Path.Combine without also passing through GetFullPath.
*/
class ZipSlipGuard extends Guard {
ZipSlipGuard() { this instanceof SanitizerMethodCall }
Expr getFilePathArgument() { result = this.(SanitizerMethodCall).getFilePathArgument() }
}
private abstract class SanitizerMethodCall extends MethodCall {
SanitizerMethodCall() { this instanceof MethodCall }
abstract Expr getFilePathArgument();
}
/**
* SanitizedGuardTaintTrackingConfiguration - A Taint Tracking configuration class to trace
* parameters of a function to calls to RootSanitizerMethodCall (String.StartsWith).
*
* For example, the following function:
* void exampleFn(String somePath){
* somePath = Path.GetFullPath(somePath);
* ...
* if(somePath.startsWith("aaaaa"))
* ...
* ...
* }
*/
class SanitizedGuardTaintTrackingConfiguration extends TaintTracking2::Configuration {
SanitizedGuardTaintTrackingConfiguration() { this = "SanitizedGuardTaintTrackingConfiguration" }
override predicate isSource(DataFlow::Node source) { source instanceof DataFlow::ParameterNode }
override predicate isSink(DataFlow::Node sink) {
exists(RootSanitizerMethodCall smc |
smc.getAnArgument() = sink.asExpr() or
smc.getQualifier() = sink.asExpr()
)
}
}
/**
* An AbstractWrapperSanitizerMethod is a Method that
* is a suitable sanitizer for a ZipSlip path that may not have been canonicalized prior.
*
* If the return value of this Method correctly validates if a file path is in a valid location,
* or is a restricted subset of that validation, then any use of this Method is as valid as the Root
* sanitizer (Path.StartsWith).
*/
private abstract class AbstractWrapperSanitizerMethod extends AbstractSanitizerMethod {
Parameter paramFilename;
AbstractWrapperSanitizerMethod() {
this.getReturnType() instanceof BoolType and
this.getAParameter() = paramFilename
}
Parameter paramFilePath() { result = paramFilename }
}
/**
* A DirectWrapperSantizierMethod is a Method where
* The function can /only/ returns true when passes through the RootSanitizerGuard
*
* bool wrapperFn(a,b){
* if(guard(a,b))
* return true
* ....
* return false
* }
*
* bool wrapperFn(a,b){
* ...
* return guard(a,b)
* }
*/
class DirectWrapperSantizierMethod extends AbstractWrapperSanitizerMethod {
/**
* To be declared a Wrapper, a function must:
* - Be a predicate (return a boolean)
* - Accept and use a parameter which represents a File path
* - Contain a call to another sanitizer
* - And can only return true if the sanitizer also returns true.
*/
DirectWrapperSantizierMethod() {
// For every return statement in this Method,
forex(ReturnStmt ret | ret.getEnclosingCallable() = this |
// The function returns false (Fails the Guard)
ret.getExpr().(BoolLiteral).getBoolValue() = false
or
// It passes the guard, contraining the function argument to the Guard argument.
exists(ZipSlipGuard g, SanitizedGuardTaintTrackingConfiguration taintTracker |
g.getEnclosingCallable() = this and
taintTracker
.hasFlow(DataFlow::parameterNode(paramFilename),
DataFlow::exprNode(g.getFilePathArgument())) and
(
exists(AbstractValues::BooleanValue bv |
// If there exists a control block that guards against misuse
bv.getValue() = true and
g.controlsNode(ret.getAControlFlowNode(), bv)
)
or
// Or if the function returns the resultant of the guard call
DataFlow::localFlow(DataFlow::exprNode(g), DataFlow::exprNode(ret.getExpr()))
)
)
)
}
}
/**
* An IndirectOverloadedWrapperSanitizerMethod is a Method in which simply wraps /another/ wrapper.class
*
* Usually this will look like the following stanza:
* boolean someWrapper(string s){
* return someWrapper(s, true);
* }
*/
class IndirectOverloadedWrapperSantizierMethod extends AbstractWrapperSanitizerMethod {
/**
* To be declared a Wrapper, a function must:
* - Be a predicate (return a boolean)
* - Accept and use a parameter which represents a File path (via delegation)
* - Contain a call to another sanitizer (via delegation)
* - And can only return true if the delegate sanitizer also returns true.
*/
IndirectOverloadedWrapperSantizierMethod() {
// For every return statement in our Method,
forex(ReturnStmt ret | ret.getEnclosingCallable() = this |
// The Return statement returns false OR
ret.getExpr().(BoolLiteral).getBoolValue() = false
or
// The Method returns the result of calling another known-good sanitizer, connecting
// the parameters of this function to the sanitizer MethodCall.
exists(ZipSlipGuard g |
// If the parameter flows directly to SanitizerMethodCall, and the resultant is returned
DataFlow::localFlow(DataFlow::parameterNode(paramFilename),
DataFlow::exprNode(g.getFilePathArgument())) and
DataFlow::localFlow(DataFlow::exprNode(g), DataFlow::exprNode(ret.getExpr()))
)
)
}
}
/**
* A Wrapped Sanitizer Method call (some function that is equally or more restrictive than our root sanitizer)
*
* bool wrapperMethod(string path){
* return realSanitizer(path);
* }
*/
class WrapperSanitizerMethodCall extends SanitizerMethodCall {
AbstractWrapperSanitizerMethod wrapperMethod;
WrapperSanitizerMethodCall() {
exists(AbstractWrapperSanitizerMethod sm |
this.getTarget() = sm and
wrapperMethod = sm
)
}
override Expr getFilePathArgument() {
result = this.getArgument(wrapperMethod.paramFilePath().getIndex())
}
}
private predicate wrapperCheckGuard(Guard g, Expr e, AbstractValue v) {
// A given wrapper method call, with the filePathArgument as a sink, that returns 'true'
g instanceof WrapperSanitizerMethodCall and
g.(WrapperSanitizerMethodCall).getFilePathArgument() = e and
v.(AbstractValues::BooleanValue).getValue() = true
}
/**
* A sanitizer for unsafe zip extraction.
*/
private abstract class Sanitizer extends DataFlow::ExprNode { }
class WrapperCheckSanitizer extends Sanitizer {
// A Wrapped RootSanitizer that is an explicit subset of RootSanitizer
WrapperCheckSanitizer() { this = DataFlow::BarrierGuard<wrapperCheckGuard/3>::getABarrierNode() }
}
/**
* A data flow source for unsafe zip extraction.
*/
private abstract class Source extends DataFlow::Node { }
/**
* Access to the `FullName` property of the archive item
*/
class ArchiveEntryFullName extends Source {
ArchiveEntryFullName() {
exists(PropertyAccess pa |
pa.getTarget().hasQualifiedName("System.IO.Compression.ZipArchiveEntry", "FullName") and
this = DataFlow::exprNode(pa)
)
}
}
/**
* A data flow sink for unsafe zip extraction.
*/
private abstract class Sink extends DataFlow::Node { }
/**
* Argument to extract to file extension method
*/
class SinkCompressionExtractToFileArgument extends Sink {
SinkCompressionExtractToFileArgument() {
exists(MethodCall mc |
mc.getTarget().hasQualifiedName("System.IO.Compression", "ZipFileExtensions", "ExtractToFile") and
mc.getTarget().hasQualifiedName("System.IO.Compression.ZipFileExtensions", "ExtractToFile") and
this.asExpr() = mc.getArgumentForName("destinationFileName")
)
}
}
/** A path argument to a `File.Open`, `File.OpenWrite`, or `File.Create` method call. */
class FileOpenArgSink extends Sink {
FileOpenArgSink() {
/**
* File Stream created from tainted file name through File.Open/File.Create
*/
class SinkFileOpenArgument extends Sink {
SinkFileOpenArgument() {
exists(MethodCall mc |
mc.getTarget().hasQualifiedName("System.IO", "File", "Open") or
mc.getTarget().hasQualifiedName("System.IO", "File", "OpenWrite") or
mc.getTarget().hasQualifiedName("System.IO", "File", "Create")
|
(
mc.getTarget().hasQualifiedName("System.IO.File", "Open") or
mc.getTarget().hasQualifiedName("System.IO.File", "OpenWrite") or
mc.getTarget().hasQualifiedName("System.IO.File", "Create")
) and
this.asExpr() = mc.getArgumentForName("path")
)
}
}
/** A path argument to a call to the `FileStream` constructor. */
class FileStreamArgSink extends Sink {
FileStreamArgSink() {
/**
* File Stream created from tainted file name passed directly to the constructor
*/
class SinkStreamConstructorArgument extends Sink {
SinkStreamConstructorArgument() {
exists(ObjectCreation oc |
oc.getTarget().getDeclaringType().hasQualifiedName("System.IO", "FileStream")
|
oc.getTarget().getDeclaringType().hasQualifiedName("System.IO", "FileStream") and
this.asExpr() = oc.getArgumentForName("path")
)
}
}
/**
* A path argument to a call to the `FileStream` constructor.
*
* This constructor can accept a tainted file name and subsequently be used to open a file stream.
* Constructor to FileInfo can take tainted file name and subsequently be used to open file stream
*/
class FileInfoArgSink extends Sink {
FileInfoArgSink() {
class SinkFileInfoConstructorArgument extends Sink {
SinkFileInfoConstructorArgument() {
exists(ObjectCreation oc |
oc.getTarget().getDeclaringType().hasQualifiedName("System.IO", "FileInfo")
|
oc.getTarget().getDeclaringType().hasQualifiedName("System.IO", "FileInfo") and
this.asExpr() = oc.getArgumentForName("fileName")
)
}
}
/**
* A call to `GetFileName`.
*
* This is considered a sanitizer because it extracts just the file name, not the full path.
* Extracting just file name from a ZipEntry, not the full path
*/
class GetFileNameSanitizer extends Sanitizer {
GetFileNameSanitizer() {
exists(MethodCall mc | mc.getTarget().hasQualifiedName("System.IO", "Path", "GetFileName") |
this.asExpr() = mc
class FileNameExtrationSanitizer extends Sanitizer {
FileNameExtrationSanitizer() {
exists(MethodCall mc |
mc.getTarget().hasQualifiedName("System.IO.Path", "GetFileName") and
this = DataFlow::exprNode(mc.getAnArgument())
)
}
}
/**
* A call to `Substring`.
*
* This is considered a sanitizer because `Substring` may be used to extract a single component
* of a path to avoid ZipSlip.
*/
class SubstringSanitizer extends Sanitizer {
SubstringSanitizer() {
exists(MethodCall mc | mc.getTarget().hasQualifiedName("System", "String", "Substring") |
this.asExpr() = mc
)
}
}
private predicate stringCheckGuard(Guard g, Expr e, AbstractValue v) {
g.(MethodCall).getTarget().hasQualifiedName("System", "String", "StartsWith") and
g.(MethodCall).getQualifier() = e and
// A StartsWith check against Path.Combine is not sufficient, because the ".." elements have
// not yet been resolved.
not exists(MethodCall combineCall |
combineCall.getTarget().hasQualifiedName("System.IO", "Path", "Combine") and
DataFlow::localExprFlow(combineCall, e)
) and
v.(AbstractValues::BooleanValue).getValue() = true
}
/**
* A call to `String.StartsWith()` that indicates that the tainted path value is being
* validated to ensure that it occurs within a permitted output path.
* Checks the string for relative path,
* or checks the destination folder for whitelisted/target path, etc
*/
class StringCheckSanitizer extends Sanitizer {
StringCheckSanitizer() { this = DataFlow::BarrierGuard<stringCheckGuard/3>::getABarrierNode() }
StringCheckSanitizer() {
exists(MethodCall mc |
(
mc instanceof RootSanitizerMethodCall or
mc.getTarget().hasQualifiedName("System.String", "Substring")
) and
this = DataFlow::exprNode(mc.getQualifier())
)
}
}
final class ZipSlipTaintTrackingConfiguration extends TaintTracking::Configuration {
ZipSlipTaintTrackingConfiguration() { this = "ZipSlipTaintTrackingConfiguration" }
override predicate isSource(DataFlow::Node node) { node instanceof Source }
override predicate isSink(DataFlow::Node node) { node instanceof Sink }
override predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
super.isAdditionalTaintStep(pred, succ)
or
exists(MethodCall mc | succ.asExpr() = mc and pred.asExpr() = mc.getAnArgument())
}
override predicate isSanitizer(DataFlow::Node node) { node instanceof Sanitizer }
}

View File

@@ -27,7 +27,7 @@ written to <code>c:\sneaky-file</code>.</p>
<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:</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

View File

@@ -16,8 +16,8 @@ import csharp
import semmle.code.csharp.security.dataflow.ZipSlipQuery
import semmle.code.csharp.dataflow.DataFlow::DataFlow::PathGraph
from TaintTrackingConfiguration zipTaintTracking, DataFlow::PathNode source, DataFlow::PathNode sink
from ZipSlipTaintTrackingConfiguration zipTaintTracking, DataFlow::PathNode source, DataFlow::PathNode sink
where zipTaintTracking.hasFlowPath(source, sink)
select source.getNode(), source, sink,
"Unsanitized archive entry, which may contain '..', is used in a $@.", sink.getNode(),
"file system operation"
"Unsanitized archive entry, which may contain '..', is used in a $@.", sink.getNode(),
"file system operation"

View File

@@ -6,44 +6,88 @@ namespace ZipSlip
{
class Program
{
private static readonly char DirectorySeparatorChar = '\\';
public static void UnzipFileByFile(ZipArchive archive,
string destDirectory)
public static void UnzipFileByFile(ZipArchive archive, string destDirectory)
{
foreach (var entry in archive.Entries)
{
string fullPath = Path.GetFullPath(entry.FullName);
string fileName = Path.GetFileName(entry.FullName);
string filename = entry.Name;
string file = entry.FullName;
if (!string.IsNullOrEmpty(file))
string fullPath_relative = Path.GetFullPath(entry.FullName);
string filename_filenameOnly = Path.GetFileName(entry.FullName);
string filename_noPathTraversal = entry.Name;
string file_badDirectoryTraversal = entry.FullName;
if (!string.IsNullOrEmpty(file_badDirectoryTraversal))
{
// BAD
string destFileName = Path.Combine(destDirectory, file);
string destFileName = Path.Combine(destDirectory, file_badDirectoryTraversal);
entry.ExtractToFile(destFileName, true);
// GOOD
string sanitizedFileName = Path.Combine(destDirectory, fileName);
string sanitizedFileName = Path.Combine(destDirectory, filename_filenameOnly);
entry.ExtractToFile(sanitizedFileName, true);
// BAD
string destFilePath = Path.Combine(destDirectory, fullPath);
string destFilePath = Path.Combine(destDirectory, fullPath_relative);
entry.ExtractToFile(destFilePath, true);
// BAD: destFilePath isn't fully resolved, so may still contain ..
if (destFilePath.StartsWith(destDirectory))
entry.ExtractToFile(destFilePath, true);
unzipWrapperProtected(destDirectory, entry);
// BAD
destFilePath = Path.GetFullPath(Path.Combine(destDirectory, fullPath));
entry.ExtractToFile(destFilePath, true);
string destFilePath_notCanonicalized = destDirectory + "/" + fullPath_relative;
if (destFilePath_notCanonicalized.StartsWith(destDirectory)){
// BAD: no canonicalization has been applied. Directory traversal characters
// could still be present ie C:\some\dir\..\..\abc.exe
entry.ExtractToFile(destFilePath_notCanonicalized, true);
}
// GOOD: a check for StartsWith against a fully resolved path
if (destFilePath.StartsWith(destDirectory))
entry.ExtractToFile(destFilePath, true);
string destFilePath_fullyCanonicalized = Path.GetFullPath(destFilePath_notCanonicalized);
if (destFilePath_fullyCanonicalized.StartsWith(destDirectory)){
// GOOD: canonicalization has been applied by GetFullPath, +StartsWith Barrier.
entry.ExtractToFile(destFilePath_fullyCanonicalized, true);
}
string destFilePath_fullyCanonicalized2 = Path.GetFullPath(destFileName);
if (destFilePath_fullyCanonicalized2.StartsWith(destDirectory)){
// GOOD: canonicalization has been applied by GetFullPath, +StartsWith Barrier.
entry.ExtractToFile(destFilePath_fullyCanonicalized2, true);
}
}
}
}
private static void unzipWrapperProtected(string destinationPath, ZipArchiveEntry entry){
string fullpath = Path.Combine(destinationPath, entry.FullName);
string entry_fullpath = Path.GetFullPath(entry.FullName);
// BAD: no canonicalization, no validation/guard.
entry.ExtractToFile(fullpath, true);
if(ContainsPath(fullpath, destinationPath, true)){
// GOOD - Barrier guard applied (canonicalization applied in ContainsPath)
entry.ExtractToFile(fullpath, true);
}
if(!ContainsPath(fullpath, destinationPath, true)){
// BAD: Failed guard
entry.ExtractToFile(fullpath, true);
Console.WriteLine("Path traversal detected");
return;
}
// GOOD: Path has been sanitized above and guarded for (by returning early)
entry.ExtractToFile(fullpath, true);
if(ContainsPath(fullpath, destinationPath, true)){
// GOOD: guarded by ContainsPath (with delegate calls to StartsWith)
entry.ExtractToFile(fullpath, true);
}
// GOOD: path checking applied above (and function terminates early).
string destFilePath = Path.Combine(destinationPath, entry_fullpath);
if (!destFilePath.StartsWith(destinationPath)){
return;
}
entry.ExtractToFile(fullpath, true);
}
private static int UnzipToStream(Stream zipStream, string installDir)
{
@@ -115,6 +159,43 @@ namespace ZipSlip
return returnCode;
}
public static string? AddBackslashIfNotPresent(string? path)
{
if (!string.IsNullOrEmpty(path) && path![path.Length - 1] != DirectorySeparatorChar)
{
path += DirectorySeparatorChar;
}
return path;
}
public static bool ContainsPath(string? fullPath, string? path){
return ContainsPath(fullPath, path, true);
}
public static bool ContainsPath(string? fullPath, string? path, bool excludeSame)
{
try
{
fullPath = Path.GetFullPath(fullPath);
path = Path.GetFullPath(path);
fullPath = AddBackslashIfNotPresent(fullPath);
path = AddBackslashIfNotPresent(path);
var result = fullPath!.StartsWith(path, StringComparison.OrdinalIgnoreCase);
if (result && excludeSame)
{
return !fullPath.Equals(path, StringComparison.OrdinalIgnoreCase);
}
return result;
}
catch
{
// If there is any error, just return false
return false;
}
}
static void Main(string[] args)
{
string zipFileName;

View File

@@ -1,55 +1,67 @@
edges
| ZipSlip.cs:15:35:15:66 | call to method GetFullPath : String | ZipSlip.cs:30:71:30:78 | access to local variable fullPath : String |
| ZipSlip.cs:15:35:15:66 | call to method GetFullPath : String | ZipSlip.cs:38:81:38:88 | access to local variable fullPath : String |
| ZipSlip.cs:15:52:15:65 | access to property FullName : String | ZipSlip.cs:15:35:15:66 | call to method GetFullPath : String |
| ZipSlip.cs:18:31:18:44 | access to property FullName : String | ZipSlip.cs:22:71:22:74 | access to local variable file : String |
| ZipSlip.cs:22:43:22:75 | call to method Combine : String | ZipSlip.cs:23:41:23:52 | access to local variable destFileName |
| ZipSlip.cs:22:71:22:74 | access to local variable file : String | ZipSlip.cs:22:43:22:75 | call to method Combine : String |
| ZipSlip.cs:30:43:30:79 | call to method Combine : String | ZipSlip.cs:31:41:31:52 | access to local variable destFilePath |
| ZipSlip.cs:30:43:30:79 | call to method Combine : String | ZipSlip.cs:35:45:35:56 | access to local variable destFilePath |
| ZipSlip.cs:30:71:30:78 | access to local variable fullPath : String | ZipSlip.cs:30:43:30:79 | call to method Combine : String |
| ZipSlip.cs:38:36:38:90 | call to method GetFullPath : String | ZipSlip.cs:39:41:39:52 | access to local variable destFilePath |
| ZipSlip.cs:38:53:38:89 | call to method Combine : String | ZipSlip.cs:38:36:38:90 | call to method GetFullPath : String |
| ZipSlip.cs:38:81:38:88 | access to local variable fullPath : String | ZipSlip.cs:38:53:38:89 | call to method Combine : String |
| ZipSlip.cs:61:47:61:86 | call to method Combine : String | ZipSlip.cs:68:74:68:85 | access to local variable destFilePath |
| ZipSlip.cs:61:47:61:86 | call to method Combine : String | ZipSlip.cs:75:71:75:82 | access to local variable destFilePath |
| ZipSlip.cs:61:47:61:86 | call to method Combine : String | ZipSlip.cs:82:57:82:68 | access to local variable destFilePath |
| ZipSlip.cs:61:47:61:86 | call to method Combine : String | ZipSlip.cs:90:58:90:69 | access to local variable destFilePath |
| ZipSlip.cs:61:72:61:85 | access to property FullName : String | ZipSlip.cs:61:47:61:86 | call to method Combine : String |
| ZipSlip.cs:15:44:15:75 | call to method GetFullPath : String | ZipSlip.cs:30:71:30:87 | access to local variable fullPath_relative : String |
| ZipSlip.cs:15:44:15:75 | call to method GetFullPath : String | ZipSlip.cs:31:41:31:52 | access to local variable destFilePath |
| ZipSlip.cs:15:44:15:75 | call to method GetFullPath : String | ZipSlip.cs:39:45:39:73 | access to local variable destFilePath_notCanonicalized |
| ZipSlip.cs:15:61:15:74 | access to property FullName : String | ZipSlip.cs:15:44:15:75 | call to method GetFullPath : String |
| ZipSlip.cs:15:61:15:74 | access to property FullName : String | ZipSlip.cs:30:71:30:87 | access to local variable fullPath_relative : String |
| ZipSlip.cs:15:61:15:74 | access to property FullName : String | ZipSlip.cs:31:41:31:52 | access to local variable destFilePath |
| ZipSlip.cs:15:61:15:74 | access to property FullName : String | ZipSlip.cs:39:45:39:73 | access to local variable destFilePath_notCanonicalized |
| ZipSlip.cs:18:53:18:66 | access to property FullName : String | ZipSlip.cs:22:71:22:96 | access to local variable file_badDirectoryTraversal : String |
| ZipSlip.cs:18:53:18:66 | access to property FullName : String | ZipSlip.cs:23:41:23:52 | access to local variable destFileName |
| ZipSlip.cs:22:43:22:97 | call to method Combine : String | ZipSlip.cs:23:41:23:52 | access to local variable destFileName |
| ZipSlip.cs:22:71:22:96 | access to local variable file_badDirectoryTraversal : String | ZipSlip.cs:22:43:22:97 | call to method Combine : String |
| ZipSlip.cs:30:43:30:88 | call to method Combine : String | ZipSlip.cs:31:41:31:52 | access to local variable destFilePath |
| ZipSlip.cs:30:71:30:87 | access to local variable fullPath_relative : String | ZipSlip.cs:30:43:30:88 | call to method Combine : String |
| ZipSlip.cs:58:31:58:75 | call to method Combine : String | ZipSlip.cs:62:33:62:40 | access to local variable fullpath |
| ZipSlip.cs:58:31:58:75 | call to method Combine : String | ZipSlip.cs:71:37:71:44 | access to local variable fullpath |
| ZipSlip.cs:58:61:58:74 | access to property FullName : String | ZipSlip.cs:58:31:58:75 | call to method Combine : String |
| ZipSlip.cs:58:61:58:74 | access to property FullName : String | ZipSlip.cs:62:33:62:40 | access to local variable fullpath |
| ZipSlip.cs:58:61:58:74 | access to property FullName : String | ZipSlip.cs:71:37:71:44 | access to local variable fullpath |
| ZipSlip.cs:105:47:105:86 | call to method Combine : String | ZipSlip.cs:112:74:112:85 | access to local variable destFilePath |
| ZipSlip.cs:105:47:105:86 | call to method Combine : String | ZipSlip.cs:119:71:119:82 | access to local variable destFilePath |
| ZipSlip.cs:105:47:105:86 | call to method Combine : String | ZipSlip.cs:126:57:126:68 | access to local variable destFilePath |
| ZipSlip.cs:105:47:105:86 | call to method Combine : String | ZipSlip.cs:134:58:134:69 | access to local variable destFilePath |
| ZipSlip.cs:105:72:105:85 | access to property FullName : String | ZipSlip.cs:105:47:105:86 | call to method Combine : String |
| ZipSlip.cs:105:72:105:85 | access to property FullName : String | ZipSlip.cs:112:74:112:85 | access to local variable destFilePath |
| ZipSlip.cs:105:72:105:85 | access to property FullName : String | ZipSlip.cs:119:71:119:82 | access to local variable destFilePath |
| ZipSlip.cs:105:72:105:85 | access to property FullName : String | ZipSlip.cs:126:57:126:68 | access to local variable destFilePath |
| ZipSlip.cs:105:72:105:85 | access to property FullName : String | ZipSlip.cs:134:58:134:69 | access to local variable destFilePath |
| ZipSlipBad.cs:9:31:9:73 | call to method Combine : String | ZipSlipBad.cs:10:29:10:40 | access to local variable destFileName |
| ZipSlipBad.cs:9:59:9:72 | access to property FullName : String | ZipSlipBad.cs:9:31:9:73 | call to method Combine : String |
| ZipSlipBad.cs:9:59:9:72 | access to property FullName : String | ZipSlipBad.cs:10:29:10:40 | access to local variable destFileName |
nodes
| ZipSlip.cs:15:35:15:66 | call to method GetFullPath : String | semmle.label | call to method GetFullPath : String |
| ZipSlip.cs:15:52:15:65 | access to property FullName : String | semmle.label | access to property FullName : String |
| ZipSlip.cs:18:31:18:44 | access to property FullName : String | semmle.label | access to property FullName : String |
| ZipSlip.cs:22:43:22:75 | call to method Combine : String | semmle.label | call to method Combine : String |
| ZipSlip.cs:22:71:22:74 | access to local variable file : String | semmle.label | access to local variable file : String |
| ZipSlip.cs:15:44:15:75 | call to method GetFullPath : String | semmle.label | call to method GetFullPath : String |
| ZipSlip.cs:15:61:15:74 | access to property FullName : String | semmle.label | access to property FullName : String |
| ZipSlip.cs:18:53:18:66 | access to property FullName : String | semmle.label | access to property FullName : String |
| ZipSlip.cs:22:43:22:97 | call to method Combine : String | semmle.label | call to method Combine : String |
| ZipSlip.cs:22:71:22:96 | access to local variable file_badDirectoryTraversal : String | semmle.label | access to local variable file_badDirectoryTraversal : String |
| ZipSlip.cs:23:41:23:52 | access to local variable destFileName | semmle.label | access to local variable destFileName |
| ZipSlip.cs:30:43:30:79 | call to method Combine : String | semmle.label | call to method Combine : String |
| ZipSlip.cs:30:71:30:78 | access to local variable fullPath : String | semmle.label | access to local variable fullPath : String |
| ZipSlip.cs:30:43:30:88 | call to method Combine : String | semmle.label | call to method Combine : String |
| ZipSlip.cs:30:71:30:87 | access to local variable fullPath_relative : String | semmle.label | access to local variable fullPath_relative : String |
| ZipSlip.cs:31:41:31:52 | access to local variable destFilePath | semmle.label | access to local variable destFilePath |
| ZipSlip.cs:35:45:35:56 | access to local variable destFilePath | semmle.label | access to local variable destFilePath |
| ZipSlip.cs:38:36:38:90 | call to method GetFullPath : String | semmle.label | call to method GetFullPath : String |
| ZipSlip.cs:38:53:38:89 | call to method Combine : String | semmle.label | call to method Combine : String |
| ZipSlip.cs:38:81:38:88 | access to local variable fullPath : String | semmle.label | access to local variable fullPath : String |
| ZipSlip.cs:39:41:39:52 | access to local variable destFilePath | semmle.label | access to local variable destFilePath |
| ZipSlip.cs:61:47:61:86 | call to method Combine : String | semmle.label | call to method Combine : String |
| ZipSlip.cs:61:72:61:85 | access to property FullName : String | semmle.label | access to property FullName : String |
| ZipSlip.cs:68:74:68:85 | access to local variable destFilePath | semmle.label | access to local variable destFilePath |
| ZipSlip.cs:75:71:75:82 | access to local variable destFilePath | semmle.label | access to local variable destFilePath |
| ZipSlip.cs:82:57:82:68 | access to local variable destFilePath | semmle.label | access to local variable destFilePath |
| ZipSlip.cs:90:58:90:69 | access to local variable destFilePath | semmle.label | access to local variable destFilePath |
| ZipSlip.cs:39:45:39:73 | access to local variable destFilePath_notCanonicalized | semmle.label | access to local variable destFilePath_notCanonicalized |
| ZipSlip.cs:58:31:58:75 | call to method Combine : String | semmle.label | call to method Combine : String |
| ZipSlip.cs:58:61:58:74 | access to property FullName : String | semmle.label | access to property FullName : String |
| ZipSlip.cs:62:33:62:40 | access to local variable fullpath | semmle.label | access to local variable fullpath |
| ZipSlip.cs:71:37:71:44 | access to local variable fullpath | semmle.label | access to local variable fullpath |
| ZipSlip.cs:105:47:105:86 | call to method Combine : String | semmle.label | call to method Combine : String |
| ZipSlip.cs:105:72:105:85 | access to property FullName : String | semmle.label | access to property FullName : String |
| ZipSlip.cs:112:74:112:85 | access to local variable destFilePath | semmle.label | access to local variable destFilePath |
| ZipSlip.cs:119:71:119:82 | access to local variable destFilePath | semmle.label | access to local variable destFilePath |
| ZipSlip.cs:126:57:126:68 | access to local variable destFilePath | semmle.label | access to local variable destFilePath |
| ZipSlip.cs:134:58:134:69 | access to local variable destFilePath | semmle.label | access to local variable destFilePath |
| ZipSlipBad.cs:9:31:9:73 | call to method Combine : String | semmle.label | call to method Combine : String |
| ZipSlipBad.cs:9:59:9:72 | access to property FullName : String | semmle.label | access to property FullName : String |
| ZipSlipBad.cs:10:29:10:40 | access to local variable destFileName | semmle.label | access to local variable destFileName |
subpaths
#select
| ZipSlip.cs:15:52:15:65 | access to property FullName | ZipSlip.cs:15:52:15:65 | access to property FullName : String | ZipSlip.cs:31:41:31:52 | access to local variable destFilePath | Unsanitized archive entry, which may contain '..', is used in a $@. | ZipSlip.cs:31:41:31:52 | access to local variable destFilePath | file system operation |
| ZipSlip.cs:15:52:15:65 | access to property FullName | ZipSlip.cs:15:52:15:65 | access to property FullName : String | ZipSlip.cs:35:45:35:56 | access to local variable destFilePath | Unsanitized archive entry, which may contain '..', is used in a $@. | ZipSlip.cs:35:45:35:56 | access to local variable destFilePath | file system operation |
| ZipSlip.cs:15:52:15:65 | access to property FullName | ZipSlip.cs:15:52:15:65 | access to property FullName : String | ZipSlip.cs:39:41:39:52 | access to local variable destFilePath | Unsanitized archive entry, which may contain '..', is used in a $@. | ZipSlip.cs:39:41:39:52 | access to local variable destFilePath | file system operation |
| ZipSlip.cs:18:31:18:44 | access to property FullName | ZipSlip.cs:18:31:18:44 | access to property FullName : String | ZipSlip.cs:23:41:23:52 | access to local variable destFileName | Unsanitized archive entry, which may contain '..', is used in a $@. | ZipSlip.cs:23:41:23:52 | access to local variable destFileName | file system operation |
| ZipSlip.cs:61:72:61:85 | access to property FullName | ZipSlip.cs:61:72:61:85 | access to property FullName : String | ZipSlip.cs:68:74:68:85 | access to local variable destFilePath | Unsanitized archive entry, which may contain '..', is used in a $@. | ZipSlip.cs:68:74:68:85 | access to local variable destFilePath | file system operation |
| ZipSlip.cs:61:72:61:85 | access to property FullName | ZipSlip.cs:61:72:61:85 | access to property FullName : String | ZipSlip.cs:75:71:75:82 | access to local variable destFilePath | Unsanitized archive entry, which may contain '..', is used in a $@. | ZipSlip.cs:75:71:75:82 | access to local variable destFilePath | file system operation |
| ZipSlip.cs:61:72:61:85 | access to property FullName | ZipSlip.cs:61:72:61:85 | access to property FullName : String | ZipSlip.cs:82:57:82:68 | access to local variable destFilePath | Unsanitized archive entry, which may contain '..', is used in a $@. | ZipSlip.cs:82:57:82:68 | access to local variable destFilePath | file system operation |
| ZipSlip.cs:61:72:61:85 | access to property FullName | ZipSlip.cs:61:72:61:85 | access to property FullName : String | ZipSlip.cs:90:58:90:69 | access to local variable destFilePath | Unsanitized archive entry, which may contain '..', is used in a $@. | ZipSlip.cs:90:58:90:69 | access to local variable destFilePath | file system operation |
| ZipSlip.cs:15:61:15:74 | access to property FullName | ZipSlip.cs:15:61:15:74 | access to property FullName : String | ZipSlip.cs:31:41:31:52 | access to local variable destFilePath | Unsanitized archive entry, which may contain '..', is used in a $@. | ZipSlip.cs:31:41:31:52 | access to local variable destFilePath | file system operation |
| ZipSlip.cs:15:61:15:74 | access to property FullName | ZipSlip.cs:15:61:15:74 | access to property FullName : String | ZipSlip.cs:39:45:39:73 | access to local variable destFilePath_notCanonicalized | Unsanitized archive entry, which may contain '..', is used in a $@. | ZipSlip.cs:39:45:39:73 | access to local variable destFilePath_notCanonicalized | file system operation |
| ZipSlip.cs:18:53:18:66 | access to property FullName | ZipSlip.cs:18:53:18:66 | access to property FullName : String | ZipSlip.cs:23:41:23:52 | access to local variable destFileName | Unsanitized archive entry, which may contain '..', is used in a $@. | ZipSlip.cs:23:41:23:52 | access to local variable destFileName | file system operation |
| ZipSlip.cs:58:61:58:74 | access to property FullName | ZipSlip.cs:58:61:58:74 | access to property FullName : String | ZipSlip.cs:62:33:62:40 | access to local variable fullpath | Unsanitized archive entry, which may contain '..', is used in a $@. | ZipSlip.cs:62:33:62:40 | access to local variable fullpath | file system operation |
| ZipSlip.cs:58:61:58:74 | access to property FullName | ZipSlip.cs:58:61:58:74 | access to property FullName : String | ZipSlip.cs:71:37:71:44 | access to local variable fullpath | Unsanitized archive entry, which may contain '..', is used in a $@. | ZipSlip.cs:71:37:71:44 | access to local variable fullpath | file system operation |
| ZipSlip.cs:105:72:105:85 | access to property FullName | ZipSlip.cs:105:72:105:85 | access to property FullName : String | ZipSlip.cs:112:74:112:85 | access to local variable destFilePath | Unsanitized archive entry, which may contain '..', is used in a $@. | ZipSlip.cs:112:74:112:85 | access to local variable destFilePath | file system operation |
| ZipSlip.cs:105:72:105:85 | access to property FullName | ZipSlip.cs:105:72:105:85 | access to property FullName : String | ZipSlip.cs:119:71:119:82 | access to local variable destFilePath | Unsanitized archive entry, which may contain '..', is used in a $@. | ZipSlip.cs:119:71:119:82 | access to local variable destFilePath | file system operation |
| ZipSlip.cs:105:72:105:85 | access to property FullName | ZipSlip.cs:105:72:105:85 | access to property FullName : String | ZipSlip.cs:126:57:126:68 | access to local variable destFilePath | Unsanitized archive entry, which may contain '..', is used in a $@. | ZipSlip.cs:126:57:126:68 | access to local variable destFilePath | file system operation |
| ZipSlip.cs:105:72:105:85 | access to property FullName | ZipSlip.cs:105:72:105:85 | access to property FullName : String | ZipSlip.cs:134:58:134:69 | access to local variable destFilePath | Unsanitized archive entry, which may contain '..', is used in a $@. | ZipSlip.cs:134:58:134:69 | access to local variable destFilePath | file system operation |
| ZipSlipBad.cs:9:59:9:72 | access to property FullName | ZipSlipBad.cs:9:59:9:72 | access to property FullName : String | ZipSlipBad.cs:10:29:10:40 | access to local variable destFileName | Unsanitized archive entry, which may contain '..', is used in a $@. | ZipSlipBad.cs:10:29:10:40 | access to local variable destFileName | file system operation |