From e946f84de03a4d2266e4a1b0968b31a1e021c776 Mon Sep 17 00:00:00 2001 From: amammad Date: Wed, 26 Apr 2023 06:46:41 +0200 Subject: [PATCH 01/11] v1 --- .../DecompressionBomb.ql | 121 +++++++++++++++ .../DecompressionBombs.qhelp | 26 ++++ .../RemoteFlowSource.qll | 62 ++++++++ .../DecompressionBombs.cs | 140 ++++++++++++++++++ .../DecompressionBombs.qlref | 1 + 5 files changed, 350 insertions(+) create mode 100644 csharp/ql/src/experimental/CWE-502-DecompressionBombs/DecompressionBomb.ql create mode 100644 csharp/ql/src/experimental/CWE-502-DecompressionBombs/DecompressionBombs.qhelp create mode 100644 csharp/ql/src/experimental/CWE-502-DecompressionBombs/RemoteFlowSource.qll create mode 100644 csharp/ql/test/experimental/CWE-502-DecompressionBombs/DecompressionBombs.cs create mode 100644 csharp/ql/test/experimental/CWE-502-DecompressionBombs/DecompressionBombs.qlref diff --git a/csharp/ql/src/experimental/CWE-502-DecompressionBombs/DecompressionBomb.ql b/csharp/ql/src/experimental/CWE-502-DecompressionBombs/DecompressionBomb.ql new file mode 100644 index 000000000000..72f81731800a --- /dev/null +++ b/csharp/ql/src/experimental/CWE-502-DecompressionBombs/DecompressionBomb.ql @@ -0,0 +1,121 @@ +/** + * @name User-controlled file decompression + * @description User-controlled data that flows into decompression library APIs without checking the compression rate is dangerous + * @kind path-problem + * @problem.severity error + * @security-severity 7.8 + * @precision medium + * @id cs/user-controlled-file-decompression + * @tags security + * experimental + * external/cwe/cwe-409 + */ + +import csharp +import semmle.code.csharp.security.dataflow.flowsources.Remote + +/** + * A data flow source for unsafe Decompression extraction. + */ +abstract class DecompressionSource extends DataFlow::Node { } + +class ZipOpenReadSource extends DecompressionSource { + ZipOpenReadSource() { + exists(MethodCall mc | + mc.getTarget().hasQualifiedName("System.IO.Compression", "ZipFile", ["OpenRead", "Open"]) and + this.asExpr() = mc.getArgument(0) and + not mc.getArgument(0).getType().isConst() + ) + } +} + +/** A path argument to a call to the `ZipArchive` constructor call. */ +class ZipArchiveArgSource extends DecompressionSource { + ZipArchiveArgSource() { + exists(ObjectCreation oc | + oc.getTarget().getDeclaringType().hasQualifiedName("System.IO.Compression", "ZipArchive") + | + this.asExpr() = oc.getArgument(0) + ) + } +} + +/** + * A data flow sink for unsafe zip extraction. + */ +abstract class DecompressionSink extends DataFlow::Node { } + +/** A Caller of the `ExtractToFile` method. */ +class ExtractToFileCallSink extends DecompressionSink { + ExtractToFileCallSink() { + exists(MethodCall mc | + mc.getTarget().hasQualifiedName("System.IO.Compression", "ZipFileExtensions", "ExtractToFile") and + this.asExpr() = mc.getArgumentForName("source") + ) + } +} + +/** A Qualifier of the `Open()` method. */ +class OpenCallSink extends DecompressionSink { + OpenCallSink() { + exists(MethodCall mc | + mc.getTarget().hasQualifiedName("System.IO.Compression", "ZipArchiveEntry", "Open") and + this.asExpr() = mc.getQualifier() + ) + } +} + +/** A Call to the `GZipStreamSink` first arugument of Constructor Call . */ +class GZipStreamSink extends DecompressionSink, DecompressionSource { + GZipStreamSink() { + exists(Constructor mc | + mc.getDeclaringType().hasQualifiedName("System.IO.Compression", "GZipStream") and + this.asExpr() = mc.getACall().getArgument(0) + ) + } +} + +/** + * A taint tracking configuration for Decompression Bomb. + */ +private module DecompressionBombConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { + source instanceof DecompressionSource + or + source instanceof RemoteFlowSource + } + + predicate isSink(DataFlow::Node sink) { sink instanceof DecompressionSink } + + predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { + // var node2 = new ZipArchive(node1, ZipArchiveMode.Read); + exists(ObjectCreation oc | + oc.getTarget().getDeclaringType().hasQualifiedName("System.IO.Compression", "ZipArchive") and + node2.asExpr() = oc and + node1.asExpr() = oc.getArgumentForName("stream") + ) + or + // var node2 = node1.ExtractToFile("./output.txt", true) + exists(MethodCall mc | + mc.getTarget().hasQualifiedName("System.IO.Compression", "ZipFileExtensions", "ExtractToFile") and + node2.asExpr() = mc and + node1.asExpr() = mc.getArgumentForName("source") + ) + or + // var node2 = node1.OpenReadStream() + exists(MethodCall mc | + mc.getTarget().hasQualifiedName("Microsoft.AspNetCore.Http", "IFormFile", "OpenReadStream") and + node2.asExpr() = mc and + node1.asExpr() = mc.getQualifier() + ) + } +} + +module DecompressionBomb = TaintTracking::Global; + +import DecompressionBomb::PathGraph + +from DecompressionBomb::PathNode source, DecompressionBomb::PathNode sink +where DecompressionBomb::flowPath(source, sink) +select sink.getNode(), source, sink, "This file extraction depends on a $@.", source.getNode(), + "potentially untrusted source" diff --git a/csharp/ql/src/experimental/CWE-502-DecompressionBombs/DecompressionBombs.qhelp b/csharp/ql/src/experimental/CWE-502-DecompressionBombs/DecompressionBombs.qhelp new file mode 100644 index 000000000000..c3c874cddf72 --- /dev/null +++ b/csharp/ql/src/experimental/CWE-502-DecompressionBombs/DecompressionBombs.qhelp @@ -0,0 +1,26 @@ + + + +

Extracting Compressed files with any compression algorithm like gzip can cause to denial of service attacks.

+

Attackers can compress a huge file which created by repeated similiar byte and convert it to a small compressed file.

+ + + + +

When you want to decompress a user-provided compressed file you must be careful about the decompression ratio or read these files within a loop byte by byte to be able to manage the decompressed size in each cycle of the loop.

+ + + + +

A good Blog Post about decompression bombs and recommended method is already written by Gérald Barré in this blog post

+ + + +
  • +A great research to gain more impact by this kind of attack +
    • + +     +     diff --git a/csharp/ql/src/experimental/CWE-502-DecompressionBombs/RemoteFlowSource.qll b/csharp/ql/src/experimental/CWE-502-DecompressionBombs/RemoteFlowSource.qll new file mode 100644 index 000000000000..f849ab3f4a19 --- /dev/null +++ b/csharp/ql/src/experimental/CWE-502-DecompressionBombs/RemoteFlowSource.qll @@ -0,0 +1,62 @@ +import csharp +import semmle.code.csharp.security.dataflow.flowsources.Remote + +/** A data flow source of remote user input by Form File (ASP.NET unvalidated request data). */ +class FormFile extends AspNetRemoteFlowSource { + FormFile() { + exists(MethodCall mc | + mc.getTarget() + .hasQualifiedName(["Microsoft.AspNetCore.Http", "Microsoft.AspNetCore.Http.Features"], + "IFormFile", ["OpenReadStream", "ContentType", "ContentDisposition", "Name", "FileName"]) and + this.asExpr() = mc + ) + or + exists(MethodCall mc | + mc.getTarget() + .hasQualifiedName(["Microsoft.AspNetCore.Http", "Microsoft.AspNetCore.Http.Features"], + "IFormFile", "CopyTo") and + this.asParameter() = mc.getTarget().getParameter(0) + ) + or + exists(Property fa | + fa.getDeclaringType() + .hasQualifiedName(["Microsoft.AspNetCore.Http", "Microsoft.AspNetCore.Http.Features"], + "IFormFile") and + fa.hasName(["ContentType", "ContentDisposition", "Name", "FileName"]) and + this.asExpr() = fa.getAnAccess() + ) + } + + override string getSourceType() { + result = "ASP.NET unvalidated request data from multipart request" + } +} + +/** A data flow source of remote user input by Form (ASP.NET unvalidated request data). */ +class FormCollection extends AspNetRemoteFlowSource { + FormCollection() { + exists(Property fa | + fa.getDeclaringType().hasQualifiedName("Microsoft.AspNetCore.Http", "IFormCollection") and + fa.hasName("Keys") and + this.asExpr() = fa.getAnAccess() + ) + } + + override string getSourceType() { + result = "ASP.NET unvalidated request data from multipart request Form Keys" + } +} + +/** A data flow source of remote user input by Headers (ASP.NET unvalidated request data). */ +class HeaderDictionary extends AspNetRemoteFlowSource { + HeaderDictionary() { + exists(Property fa | + fa.getDeclaringType().hasQualifiedName("Microsoft.AspNetCore.Http", "IHeaderDictionary") and + this.asExpr() = fa.getAnAccess() + ) + } + + override string getSourceType() { + result = "ASP.NET unvalidated request data from Headers of request" + } +} diff --git a/csharp/ql/test/experimental/CWE-502-DecompressionBombs/DecompressionBombs.cs b/csharp/ql/test/experimental/CWE-502-DecompressionBombs/DecompressionBombs.cs new file mode 100644 index 000000000000..b73eac9bade9 --- /dev/null +++ b/csharp/ql/test/experimental/CWE-502-DecompressionBombs/DecompressionBombs.cs @@ -0,0 +1,140 @@ +using System.IO.Compression; +using Microsoft.AspNetCore.Mvc; + +// For more information on enabling Web API for empty projects, visit https://go.microsoft.com/fwlink/?LinkID=397860 + +namespace MultipartFormWebAPITest.Controllers +{ + [Route("api/[controller]")] + [ApiController] + public class ZipFile1Controller : ControllerBase + { + // POST api/ + [HttpPost] + public string Post(List files) + { + if (!Request.ContentType!.StartsWith("multipart/form-data")) + return "400"; + if (files.Count == 0) + return "400"; + foreach (var formFile in files) + { + using var readStream = formFile.OpenReadStream(); + if (readStream.Length == 0) return "400"; + ZipHelpers.Bomb3(readStream); + ZipHelpers.Bomb2(formFile.FileName); + ZipHelpers.Bomb1(formFile.FileName); + } + var tmp = Request.Form["aa"]; + var tmp2 = Request.Form.Keys; + // when we don't have only one file as body + ZipHelpers.Bomb3(Request.Body); + ZipHelpers.Bomb2(Request.Query["param1"].ToString()); + var headers = Request.Headers; + ZipHelpers.Bomb1(headers.ETag); + return "200"; + } + } +} + +internal static class ZipHelpers +{ + public static void Bomb3(Stream compressedFileStream) + { + // using FileStream compressedFileStream = File.Open(CompressedFileName, FileMode.Open); + // using FileStream outputFileStream = File.Create(DecompressedFileName); + using var decompressor = new GZipStream(compressedFileStream, CompressionMode.Decompress); + using var ms = new MemoryStream(); + decompressor.CopyTo(ms); + } + + public static void Bomb2(string filename) + { + using var zipToOpen = new FileStream(filename, FileMode.Open); + using var archive = new ZipArchive(zipToOpen, ZipArchiveMode.Read); + foreach (var entry in archive.Entries) entry.ExtractToFile("./output.txt", true); // Sensitive + } + + public static void Bomb1(string filename) + { + const long maxLength = 10 * 1024 * 1024; // 10MB + // var filename = "/home/am/0_WorkDir/Payloads/Bombs/bombs-bones-codes-BH-2016/archives/evil-headers/10GB.zip"; + using var zipFile = ZipFile.OpenRead(filename); + // Quickly check the value from the zip header + var declaredSize = zipFile.Entries.Sum(entry => entry.Length); + if (declaredSize > maxLength) + throw new Exception("Archive is too big"); + foreach (var entry in zipFile.Entries) + { + using var entryStream = entry.Open(); + // Use MaxLengthStream to ensure we don't read more than the declared length + using var maxLengthStream = new MaxLengthStream(entryStream, entry.Length); + // Be sure to use the maxLengthSteam variable to read the content of the entry, not entryStream + using var ms = new MemoryStream(); + maxLengthStream.CopyTo(ms); + } + } +} + +internal sealed class MaxLengthStream : Stream +{ + private readonly Stream _stream; + private long _length; + + public MaxLengthStream(Stream stream, long maxLength) + { + _stream = stream ?? throw new ArgumentNullException(nameof(stream)); + MaxLength = maxLength; + } + + private long MaxLength { get; } + + public override bool CanRead => _stream.CanRead; + public override bool CanSeek => false; + public override bool CanWrite => false; + public override long Length => _stream.Length; + + public override long Position + { + get => _stream.Position; + set => throw new NotSupportedException(); + } + + public override int Read(byte[] buffer, int offset, int count) + { + var result = _stream.Read(buffer, offset, count); + _length += result; + if (_length > MaxLength) + throw new Exception("Stream is larger than the maximum allowed size"); + + return result; + } + + // TODO ReadAsync + + public override void Flush() + { + throw new NotSupportedException(); + } + + public override long Seek(long offset, SeekOrigin origin) + { + throw new NotSupportedException(); + } + + public override void SetLength(long value) + { + throw new NotSupportedException(); + } + + public override void Write(byte[] buffer, int offset, int count) + { + throw new NotSupportedException(); + } + + protected override void Dispose(bool disposing) + { + _stream.Dispose(); + base.Dispose(disposing); + } +} \ No newline at end of file diff --git a/csharp/ql/test/experimental/CWE-502-DecompressionBombs/DecompressionBombs.qlref b/csharp/ql/test/experimental/CWE-502-DecompressionBombs/DecompressionBombs.qlref new file mode 100644 index 000000000000..19b7ebbb8436 --- /dev/null +++ b/csharp/ql/test/experimental/CWE-502-DecompressionBombs/DecompressionBombs.qlref @@ -0,0 +1 @@ +experimental/CWE-502-DecompressionBombs/DecompressionBomb.ql \ No newline at end of file From 5a37247dbc4753dba941cf6d664b5536b051655f Mon Sep 17 00:00:00 2001 From: amammad Date: Mon, 26 Jun 2023 16:30:20 +1000 Subject: [PATCH 02/11] update test file --- .../DecompressionBombs.cs | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/csharp/ql/test/experimental/CWE-502-DecompressionBombs/DecompressionBombs.cs b/csharp/ql/test/experimental/CWE-502-DecompressionBombs/DecompressionBombs.cs index b73eac9bade9..8dbd665d7866 100644 --- a/csharp/ql/test/experimental/CWE-502-DecompressionBombs/DecompressionBombs.cs +++ b/csharp/ql/test/experimental/CWE-502-DecompressionBombs/DecompressionBombs.cs @@ -1,7 +1,6 @@ using System.IO.Compression; using Microsoft.AspNetCore.Mvc; -// For more information on enabling Web API for empty projects, visit https://go.microsoft.com/fwlink/?LinkID=397860 namespace MultipartFormWebAPITest.Controllers { @@ -23,7 +22,7 @@ public string Post(List files) if (readStream.Length == 0) return "400"; ZipHelpers.Bomb3(readStream); ZipHelpers.Bomb2(formFile.FileName); - ZipHelpers.Bomb1(formFile.FileName); + ZipHelpers.BombSafe(formFile.FileName); } var tmp = Request.Form["aa"]; var tmp2 = Request.Form.Keys; @@ -31,7 +30,7 @@ public string Post(List files) ZipHelpers.Bomb3(Request.Body); ZipHelpers.Bomb2(Request.Query["param1"].ToString()); var headers = Request.Headers; - ZipHelpers.Bomb1(headers.ETag); + ZipHelpers.BombSafe(headers.ETag); return "200"; } } @@ -41,8 +40,6 @@ internal static class ZipHelpers { public static void Bomb3(Stream compressedFileStream) { - // using FileStream compressedFileStream = File.Open(CompressedFileName, FileMode.Open); - // using FileStream outputFileStream = File.Create(DecompressedFileName); using var decompressor = new GZipStream(compressedFileStream, CompressionMode.Decompress); using var ms = new MemoryStream(); decompressor.CopyTo(ms); @@ -55,12 +52,13 @@ public static void Bomb2(string filename) foreach (var entry in archive.Entries) entry.ExtractToFile("./output.txt", true); // Sensitive } - public static void Bomb1(string filename) + public static void BombSafe(string filename) { const long maxLength = 10 * 1024 * 1024; // 10MB - // var filename = "/home/am/0_WorkDir/Payloads/Bombs/bombs-bones-codes-BH-2016/archives/evil-headers/10GB.zip"; using var zipFile = ZipFile.OpenRead(filename); - // Quickly check the value from the zip header + // Quickly check the value from the zip header + // we don't worry about malicious zip headers anymore + // because we read stream until the value of header not more var declaredSize = zipFile.Entries.Sum(entry => entry.Length); if (declaredSize > maxLength) throw new Exception("Archive is too big"); From 88cf7402c8c1335ad20039987735aa70c561ad0b Mon Sep 17 00:00:00 2001 From: amammad Date: Mon, 26 Jun 2023 21:28:59 +1000 Subject: [PATCH 03/11] change qury file name same as qhelp name --- .../{DecompressionBomb.ql => DecompressionBombs.ql} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename csharp/ql/src/experimental/CWE-502-DecompressionBombs/{DecompressionBomb.ql => DecompressionBombs.ql} (100%) diff --git a/csharp/ql/src/experimental/CWE-502-DecompressionBombs/DecompressionBomb.ql b/csharp/ql/src/experimental/CWE-502-DecompressionBombs/DecompressionBombs.ql similarity index 100% rename from csharp/ql/src/experimental/CWE-502-DecompressionBombs/DecompressionBomb.ql rename to csharp/ql/src/experimental/CWE-502-DecompressionBombs/DecompressionBombs.ql From 0bec564d36e90da04a1650d6dfff39c59b5d1e85 Mon Sep 17 00:00:00 2001 From: amammad Date: Tue, 4 Jul 2023 22:04:21 +1000 Subject: [PATCH 04/11] V2 --- .../DecompressionBombs.ql | 135 +++++++++++++----- .../RemoteFlowSource.qll | 11 +- 2 files changed, 105 insertions(+), 41 deletions(-) diff --git a/csharp/ql/src/experimental/CWE-502-DecompressionBombs/DecompressionBombs.ql b/csharp/ql/src/experimental/CWE-502-DecompressionBombs/DecompressionBombs.ql index 72f81731800a..c5884575318d 100644 --- a/csharp/ql/src/experimental/CWE-502-DecompressionBombs/DecompressionBombs.ql +++ b/csharp/ql/src/experimental/CWE-502-DecompressionBombs/DecompressionBombs.ql @@ -19,58 +19,128 @@ import semmle.code.csharp.security.dataflow.flowsources.Remote */ abstract class DecompressionSource extends DataFlow::Node { } -class ZipOpenReadSource extends DecompressionSource { - ZipOpenReadSource() { +/** + * A data flow sink for unsafe zip extraction. + */ +abstract class DecompressionSink extends DataFlow::Node { } + +class ZipFile extends DecompressionSource { + ZipFile() { exists(MethodCall mc | - mc.getTarget().hasQualifiedName("System.IO.Compression", "ZipFile", ["OpenRead", "Open"]) and - this.asExpr() = mc.getArgument(0) and + ( + mc.getTarget().hasQualifiedName("System.IO.Compression", "ZipFile", "Open") and + this.asExpr() = mc.getArgument(0) and + mc.getArgument(1) = + any(EnumConstant e | + e.hasQualifiedName("System.IO.Compression", "ZipArchiveMode", ["Update", "Read"]) + ).getAnAccess() + or + mc.getTarget().hasQualifiedName("System.IO.Compression", "ZipFile", "OpenRead") and + this.asExpr() = mc.getArgument(0) + ) and not mc.getArgument(0).getType().isConst() ) } } /** A path argument to a call to the `ZipArchive` constructor call. */ -class ZipArchiveArgSource extends DecompressionSource { - ZipArchiveArgSource() { - exists(ObjectCreation oc | - oc.getTarget().getDeclaringType().hasQualifiedName("System.IO.Compression", "ZipArchive") +class ZipArchive extends DecompressionSource { + ZipArchive() { + exists(Constructor c | + c.getDeclaringType().hasQualifiedName("System.IO.Compression", "ZipArchive") | - this.asExpr() = oc.getArgument(0) + this.asExpr() = c.getACall() and + not c.getACall().getArgument(0).getType().isConst() and + c.getACall().getArgument(1) = + any(EnumConstant e | + e.hasQualifiedName("System.IO.Compression", "ZipArchiveMode", ["Update", "Read"]) + ).getAnAccess() ) } } -/** - * A data flow sink for unsafe zip extraction. - */ -abstract class DecompressionSink extends DataFlow::Node { } - -/** A Caller of the `ExtractToFile` method. */ -class ExtractToFileCallSink extends DecompressionSink { - ExtractToFileCallSink() { +/** A Qualifier of the `Open()` method. */ +class ZipArchiveEntry extends DecompressionSink { + ZipArchiveEntry() { exists(MethodCall mc | - mc.getTarget().hasQualifiedName("System.IO.Compression", "ZipFileExtensions", "ExtractToFile") and - this.asExpr() = mc.getArgumentForName("source") + mc.getTarget().hasQualifiedName("System.IO.Compression", "ZipArchiveEntry", "Open") and + this.asExpr() = mc.getQualifier() ) } } -/** A Qualifier of the `Open()` method. */ -class OpenCallSink extends DecompressionSink { - OpenCallSink() { +/** A Caller of the `ExtractToFile` method. */ +class ZipFileExtensionsCallSink extends DecompressionSink { + ZipFileExtensionsCallSink() { exists(MethodCall mc | - mc.getTarget().hasQualifiedName("System.IO.Compression", "ZipArchiveEntry", "Open") and - this.asExpr() = mc.getQualifier() + mc.getTarget().hasQualifiedName("System.IO.Compression", "ZipFileExtensions", "ExtractToFile") and + this.asExpr() = mc.getArgumentForName("source") and + mc.getArgumentForName("source") + .getType() + .hasQualifiedName("System.IO.Compression", "ZipArchiveEntry") + or + mc.getTarget() + .hasQualifiedName("System.IO.Compression", "ZipFileExtensions", "ExtractToDirectory") and + this.asExpr() = mc.getArgumentForName("source") and + mc.getArgumentForName("source") + .getType() + .hasQualifiedName("System.IO.Compression", "ZipArchive") ) } } -/** A Call to the `GZipStreamSink` first arugument of Constructor Call . */ +/** A Call to the `GZipStream` first arugument of Constructor Call . */ class GZipStreamSink extends DecompressionSink, DecompressionSource { GZipStreamSink() { exists(Constructor mc | mc.getDeclaringType().hasQualifiedName("System.IO.Compression", "GZipStream") and - this.asExpr() = mc.getACall().getArgument(0) + this.asExpr() = mc.getACall().getArgument(0) and + mc.getACall().getArgument(1) = + any(EnumConstant e | + e.hasQualifiedName("System.IO.Compression", "CompressionMode", "Decompress") + ).getAnAccess() + ) + } +} + +/** A Call to the `BrotliStream` first arugument of Constructor Call . */ +class BrotliStreamSink extends DecompressionSink, DecompressionSource { + BrotliStreamSink() { + exists(Constructor mc | + mc.getDeclaringType().hasQualifiedName("System.IO.Compression", "BrotliStream") and + this.asExpr() = mc.getACall().getArgument(0) and + mc.getACall().getArgument(1) = + any(EnumConstant e | + e.hasQualifiedName("System.IO.Compression", "CompressionMode", "Decompress") + ).getAnAccess() + ) + } +} + +/** A Call to the `BrotliStream` first arugument of Constructor Call . */ +class DeflateStreamSink extends DecompressionSink, DecompressionSource { + DeflateStreamSink() { + exists(Constructor mc | + mc.getDeclaringType().hasQualifiedName("System.IO.Compression", "DeflateStream") and + this.asExpr() = mc.getACall().getArgument(0) and + mc.getACall().getArgument(1) = + any(EnumConstant e | + e.hasQualifiedName("System.IO.Compression", "CompressionMode", "Decompress") + ).getAnAccess() + ) + } +} + +/** A Call to the `ZLibStream` first arugument of Constructor Call . */ +class ZLibStreamSink extends DecompressionSink, DecompressionSource { + ZLibStreamSink() { + exists(Constructor mc | + mc.getDeclaringType().hasQualifiedName("System.IO.Compression", "ZLibStream") and + this.asExpr() = mc.getACall().getArgument(0) and + mc.getACall().getArgument(1) = + any(EnumConstant e | + e.hasQualifiedName("System.IO.Compression", "CompressionMode", "Decompress") + ).getAnAccess() ) } } @@ -88,13 +158,6 @@ private module DecompressionBombConfig implements DataFlow::ConfigSig { predicate isSink(DataFlow::Node sink) { sink instanceof DecompressionSink } predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { - // var node2 = new ZipArchive(node1, ZipArchiveMode.Read); - exists(ObjectCreation oc | - oc.getTarget().getDeclaringType().hasQualifiedName("System.IO.Compression", "ZipArchive") and - node2.asExpr() = oc and - node1.asExpr() = oc.getArgumentForName("stream") - ) - or // var node2 = node1.ExtractToFile("./output.txt", true) exists(MethodCall mc | mc.getTarget().hasQualifiedName("System.IO.Compression", "ZipFileExtensions", "ExtractToFile") and @@ -102,6 +165,12 @@ private module DecompressionBombConfig implements DataFlow::ConfigSig { node1.asExpr() = mc.getArgumentForName("source") ) or + exists(MethodCall mc | + mc.getTarget().hasQualifiedName("System.IO.Compression", "ZipArchive", "GetEntry") and + node1.asExpr() = mc.getQualifier() and + node2.asExpr() = mc + ) + or // var node2 = node1.OpenReadStream() exists(MethodCall mc | mc.getTarget().hasQualifiedName("Microsoft.AspNetCore.Http", "IFormFile", "OpenReadStream") and diff --git a/csharp/ql/src/experimental/CWE-502-DecompressionBombs/RemoteFlowSource.qll b/csharp/ql/src/experimental/CWE-502-DecompressionBombs/RemoteFlowSource.qll index f849ab3f4a19..337f3921adc6 100644 --- a/csharp/ql/src/experimental/CWE-502-DecompressionBombs/RemoteFlowSource.qll +++ b/csharp/ql/src/experimental/CWE-502-DecompressionBombs/RemoteFlowSource.qll @@ -5,23 +5,18 @@ import semmle.code.csharp.security.dataflow.flowsources.Remote class FormFile extends AspNetRemoteFlowSource { FormFile() { exists(MethodCall mc | - mc.getTarget() - .hasQualifiedName(["Microsoft.AspNetCore.Http", "Microsoft.AspNetCore.Http.Features"], - "IFormFile", ["OpenReadStream", "ContentType", "ContentDisposition", "Name", "FileName"]) and + mc.getTarget().hasQualifiedName("Microsoft.AspNetCore.Http", "IFormFile", "OpenReadStream") and this.asExpr() = mc ) or exists(MethodCall mc | mc.getTarget() - .hasQualifiedName(["Microsoft.AspNetCore.Http", "Microsoft.AspNetCore.Http.Features"], - "IFormFile", "CopyTo") and + .hasQualifiedName("Microsoft.AspNetCore.Http", "IFormFile", ["CopyTo", "CopyToAsync"]) and this.asParameter() = mc.getTarget().getParameter(0) ) or exists(Property fa | - fa.getDeclaringType() - .hasQualifiedName(["Microsoft.AspNetCore.Http", "Microsoft.AspNetCore.Http.Features"], - "IFormFile") and + fa.getDeclaringType().hasQualifiedName("Microsoft.AspNetCore.Http", "IFormFile") and fa.hasName(["ContentType", "ContentDisposition", "Name", "FileName"]) and this.asExpr() = fa.getAnAccess() ) From 264f15da70685145bce8b58430cf8f4ff79af323 Mon Sep 17 00:00:00 2001 From: amammad Date: Tue, 11 Jul 2023 06:11:08 +1000 Subject: [PATCH 05/11] fix qlref --- .../CWE-502-DecompressionBombs/DecompressionBombs.qlref | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/csharp/ql/test/experimental/CWE-502-DecompressionBombs/DecompressionBombs.qlref b/csharp/ql/test/experimental/CWE-502-DecompressionBombs/DecompressionBombs.qlref index 19b7ebbb8436..23c61806317b 100644 --- a/csharp/ql/test/experimental/CWE-502-DecompressionBombs/DecompressionBombs.qlref +++ b/csharp/ql/test/experimental/CWE-502-DecompressionBombs/DecompressionBombs.qlref @@ -1 +1 @@ -experimental/CWE-502-DecompressionBombs/DecompressionBomb.ql \ No newline at end of file +experimental/CWE-502-DecompressionBombs/DecompressionBombs.ql \ No newline at end of file From f68c5b345776935b62a937775b1ff645077e33d6 Mon Sep 17 00:00:00 2001 From: amammad Date: Mon, 24 Jul 2023 21:32:56 +1000 Subject: [PATCH 06/11] add SharpZipLib --- .../DecompressionBombs.ql | 86 ++++++++++++++++++- 1 file changed, 85 insertions(+), 1 deletion(-) diff --git a/csharp/ql/src/experimental/CWE-502-DecompressionBombs/DecompressionBombs.ql b/csharp/ql/src/experimental/CWE-502-DecompressionBombs/DecompressionBombs.ql index c5884575318d..ce6f4501707d 100644 --- a/csharp/ql/src/experimental/CWE-502-DecompressionBombs/DecompressionBombs.ql +++ b/csharp/ql/src/experimental/CWE-502-DecompressionBombs/DecompressionBombs.ql @@ -145,6 +145,66 @@ class ZLibStreamSink extends DecompressionSink, DecompressionSource { } } +class SharpZipLibType extends RefType { + SharpZipLibType() { + this.hasQualifiedName("ICSharpCode.SharpZipLib.Zip", "ZipInputStream") or + this.hasQualifiedName("ICSharpCode.SharpZipLib.BZip2", "BZip2InputStream") or + this.hasQualifiedName("ICSharpCode.SharpZipLib.GZip", "GZipInputStream") or + this.hasQualifiedName("ICSharpCode.SharpZipLib.Lzw", "LzwInputStream") or + this.hasQualifiedName("ICSharpCode.SharpZipLib.Tar", "TarInputStream") or + this.hasQualifiedName("ICSharpCode.SharpZipLib.Zip.Compression.Streams", "InflaterInputStream") + } +} + +class SharpZipLibSource extends DecompressionSource { + SharpZipLibSource() { + exists(Constructor c | + c.getDeclaringType() instanceof SharpZipLibType and + this.asExpr() = c.getACall() and + not c.getDeclaringType().hasQualifiedName("ICSharpCode.SharpZipLib.Tar", "TarInputStream") + ) + } +} + +class SharpZipLibDecompressionMethod extends MethodCall { + SharpZipLibDecompressionMethod() { + exists(string methodName | + methodName = + [ + "Read", "ReadAsync", "ReadAtLeast", "CopyToAsync", "CopyTo", "CopyEntryContentsAsync", + "CopyEntryContents" + ] + | + this.getTarget().hasQualifiedName("ICSharpCode.SharpZipLib.Zip", "ZipInputStream", methodName) or + this.getTarget() + .hasQualifiedName("ICSharpCode.SharpZipLib.BZip2", "BZip2InputStream", methodName) or + this.getTarget() + .hasQualifiedName("ICSharpCode.SharpZipLib.GZip", "GZipInputStream", methodName) or + this.getTarget().hasQualifiedName("ICSharpCode.SharpZipLib.Lzw", "LzwInputStream", methodName) or + this.getTarget().hasQualifiedName("ICSharpCode.SharpZipLib.Tar", "TarInputStream", methodName) or + this.getTarget() + .hasQualifiedName("ICSharpCode.SharpZipLib.Zip.Compression.Streams", + "InflaterInputStream", methodName) or + this.getTarget().hasQualifiedName("System.IO", "Stream", methodName) + ) + } +} + +class SharpZipLibSink extends DecompressionSink { + SharpZipLibSink() { + exists(SharpZipLibDecompressionMethod mc | this.asExpr() = mc.getArgument(0)) + } +} + +class SharpZipLibFastZipSink extends DecompressionSink { + SharpZipLibFastZipSink() { + exists(MethodCall mc | + mc.getTarget().hasQualifiedName("ICSharpCode.SharpZipLib.Zip", "ZipInputStream", "ExtractZip") and + this.asExpr() = mc.getArgument(0) + ) + } +} + /** * A taint tracking configuration for Decompression Bomb. */ @@ -155,7 +215,10 @@ private module DecompressionBombConfig implements DataFlow::ConfigSig { source instanceof RemoteFlowSource } - predicate isSink(DataFlow::Node sink) { sink instanceof DecompressionSink } + predicate isSink(DataFlow::Node sink) { + sink instanceof DecompressionSink + // sink instanceof DataFlow::Node + } predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { // var node2 = node1.ExtractToFile("./output.txt", true) @@ -177,6 +240,27 @@ private module DecompressionBombConfig implements DataFlow::ConfigSig { node2.asExpr() = mc and node1.asExpr() = mc.getQualifier() ) + or + exists(MethodCall mc, string methodName | + methodName = + [ + "Read", "ReadAsync", "ReadAtLeast", "CopyToAsync", "CopyTo", "CopyEntryContentsAsync", + "CopyEntryContents" + ] + | + ( + mc.getTarget().getDeclaringType() instanceof SharpZipLibType or + mc.getTarget().hasQualifiedName("System.IO", "Stream", methodName) + ) and + node1.asExpr() = mc.getQualifier() and + node2.asExpr() = mc.getArgument(0) + ) + or + exists(Constructor c | + c.getDeclaringType() instanceof SharpZipLibType and + node1.asExpr() = c.getACall().getArgument(0) and + node2.asExpr() = c.getACall() + ) } } From 1e3b9794084860805c079aea66a1a71bfbe1c10d Mon Sep 17 00:00:00 2001 From: amammad Date: Thu, 31 Aug 2023 05:51:54 +1000 Subject: [PATCH 07/11] add K4os LZ4 --- .../DecompressionBombs.ql | 38 +++++++++++++++++-- 1 file changed, 34 insertions(+), 4 deletions(-) diff --git a/csharp/ql/src/experimental/CWE-502-DecompressionBombs/DecompressionBombs.ql b/csharp/ql/src/experimental/CWE-502-DecompressionBombs/DecompressionBombs.ql index ce6f4501707d..6ee0b46c78dd 100644 --- a/csharp/ql/src/experimental/CWE-502-DecompressionBombs/DecompressionBombs.ql +++ b/csharp/ql/src/experimental/CWE-502-DecompressionBombs/DecompressionBombs.ql @@ -190,6 +190,29 @@ class SharpZipLibDecompressionMethod extends MethodCall { } } +class K4osLZ4Source extends DecompressionSource { + K4osLZ4Source() { exists(K4osLZ4InitializeMethod ma | this.asExpr() = ma.getArgument(0)) } +} + +class K4osLZ4InitializeMethod extends MethodCall { + K4osLZ4InitializeMethod() { + this.getTarget().hasQualifiedName("K4os.Compression.LZ4.Streams", "LZ4Stream", "Decode") + } +} + +class K4osLZ4DecompressionMethod extends MethodCall { + K4osLZ4DecompressionMethod() { + exists(string methodName | methodName = ["Read", "ReadAsync", "CopyToAsync", "CopyTo"] | + this.getTarget() + .hasQualifiedName("K4os.Compression.LZ4.Streams", "LZ4DecoderStream", methodName) + ) + } +} + +class K4osLZ4LibSink extends DecompressionSink { + K4osLZ4LibSink() { exists(K4osLZ4DecompressionMethod mc | this.asExpr() = mc.getArgument(0)) } +} + class SharpZipLibSink extends DecompressionSink { SharpZipLibSink() { exists(SharpZipLibDecompressionMethod mc | this.asExpr() = mc.getArgument(0)) @@ -215,10 +238,7 @@ private module DecompressionBombConfig implements DataFlow::ConfigSig { source instanceof RemoteFlowSource } - predicate isSink(DataFlow::Node sink) { - sink instanceof DecompressionSink - // sink instanceof DataFlow::Node - } + predicate isSink(DataFlow::Node sink) { sink instanceof DecompressionSink } predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { // var node2 = node1.ExtractToFile("./output.txt", true) @@ -261,6 +281,16 @@ private module DecompressionBombConfig implements DataFlow::ConfigSig { node1.asExpr() = c.getACall().getArgument(0) and node2.asExpr() = c.getACall() ) + or + exists(K4osLZ4DecompressionMethod mc | + node1.asExpr() = mc.getQualifier() and + node2.asExpr() = mc.getArgument(0) + ) + or + exists(K4osLZ4InitializeMethod mc | + node2.asExpr() = mc and + node1.asExpr() = mc.getArgument(0) + ) } } From 1476879d2b8e77e956407e8515d11669d38875c8 Mon Sep 17 00:00:00 2001 From: amammad Date: Thu, 7 Sep 2023 02:37:50 +1000 Subject: [PATCH 08/11] better warning message --- .../CWE-502-DecompressionBombs/DecompressionBombs.ql | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/csharp/ql/src/experimental/CWE-502-DecompressionBombs/DecompressionBombs.ql b/csharp/ql/src/experimental/CWE-502-DecompressionBombs/DecompressionBombs.ql index 6ee0b46c78dd..575e141cacc9 100644 --- a/csharp/ql/src/experimental/CWE-502-DecompressionBombs/DecompressionBombs.ql +++ b/csharp/ql/src/experimental/CWE-502-DecompressionBombs/DecompressionBombs.ql @@ -300,5 +300,4 @@ import DecompressionBomb::PathGraph from DecompressionBomb::PathNode source, DecompressionBomb::PathNode sink where DecompressionBomb::flowPath(source, sink) -select sink.getNode(), source, sink, "This file extraction depends on a $@.", source.getNode(), - "potentially untrusted source" +select sink.getNode(), source, sink, "This uncontrolled depends on a $@.", source.getNode(), "this" From 1f8f62833acc5b2046720d2f05ba539072674576 Mon Sep 17 00:00:00 2001 From: amammad Date: Thu, 7 Sep 2023 02:38:28 +1000 Subject: [PATCH 09/11] better warning message --- .../CWE-502-DecompressionBombs/DecompressionBombs.ql | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/csharp/ql/src/experimental/CWE-502-DecompressionBombs/DecompressionBombs.ql b/csharp/ql/src/experimental/CWE-502-DecompressionBombs/DecompressionBombs.ql index 575e141cacc9..87716a737891 100644 --- a/csharp/ql/src/experimental/CWE-502-DecompressionBombs/DecompressionBombs.ql +++ b/csharp/ql/src/experimental/CWE-502-DecompressionBombs/DecompressionBombs.ql @@ -1,6 +1,6 @@ /** - * @name User-controlled file decompression - * @description User-controlled data that flows into decompression library APIs without checking the compression rate is dangerous + * @name Uncontrolled file decompression + * @description Uncontrolled decompressionwithout checking the compression rate is dangerous * @kind path-problem * @problem.severity error * @security-severity 7.8 From 011025b8d8e91ca04e59acfd6dfd93fa717c05c1 Mon Sep 17 00:00:00 2001 From: amammad <77095239+amammad@users.noreply.github.com> Date: Tue, 17 Oct 2023 19:52:23 +0200 Subject: [PATCH 10/11] add more remote flow sources, modularize decompression bombs, remove local flow sources, improve tests --- .../security/dataflow/flowsources/Remote.qll | 6 +- .../DecompressionBombs.ql | 287 +----------------- .../RemoteFlowSource.qll | 25 +- .../dataflow/security/DecompressionBomb.qll | 273 +++++++++++++++++ .../DecompressionBombs.cs | 77 +++-- 5 files changed, 347 insertions(+), 321 deletions(-) create mode 100644 csharp/ql/src/experimental/dataflow/security/DecompressionBomb.qll diff --git a/csharp/ql/lib/semmle/code/csharp/security/dataflow/flowsources/Remote.qll b/csharp/ql/lib/semmle/code/csharp/security/dataflow/flowsources/Remote.qll index 404730ac4c4b..944830783fd8 100644 --- a/csharp/ql/lib/semmle/code/csharp/security/dataflow/flowsources/Remote.qll +++ b/csharp/ql/lib/semmle/code/csharp/security/dataflow/flowsources/Remote.qll @@ -227,14 +227,14 @@ private class AspNetCoreRemoteFlowSourceMember extends TaintTracking::TaintedMem } /** A data flow source of remote user input (ASP.NET query collection). */ -class AspNetCoreQueryRemoteFlowSource extends AspNetCoreRemoteFlowSource, DataFlow::ExprNode { +class AspNetCoreQueryRemoteFlowSource extends AspNetCoreRemoteFlowSource, DataFlow::Node { AspNetCoreQueryRemoteFlowSource() { exists(ValueOrRefType t | t instanceof MicrosoftAspNetCoreHttpHttpRequest or t instanceof MicrosoftAspNetCoreHttpQueryCollection or t instanceof MicrosoftAspNetCoreHttpQueryString | - this.getExpr().(Call).getTarget().getDeclaringType() = t or + this.asExpr().(Call).getTarget().getDeclaringType() = t or this.asExpr().(Access).getTarget().getDeclaringType() = t ) or @@ -243,7 +243,7 @@ class AspNetCoreQueryRemoteFlowSource extends AspNetCoreRemoteFlowSource, DataFl .getDeclaringType() .hasQualifiedName("Microsoft.AspNetCore.Http", "IQueryCollection") and c.getTarget().getName() = "TryGetValue" and - this.asExpr() = c.getArgumentForName("value") + this.asDefinition().getTargetAccess() = c.getArgument(1) ) } diff --git a/csharp/ql/src/experimental/CWE-502-DecompressionBombs/DecompressionBombs.ql b/csharp/ql/src/experimental/CWE-502-DecompressionBombs/DecompressionBombs.ql index 87716a737891..63bc33ba5052 100644 --- a/csharp/ql/src/experimental/CWE-502-DecompressionBombs/DecompressionBombs.ql +++ b/csharp/ql/src/experimental/CWE-502-DecompressionBombs/DecompressionBombs.ql @@ -1,6 +1,6 @@ /** * @name Uncontrolled file decompression - * @description Uncontrolled decompressionwithout checking the compression rate is dangerous + * @description Uncontrolled decompression without checking the compression rate is dangerous * @kind path-problem * @problem.severity error * @security-severity 7.8 @@ -13,291 +13,26 @@ import csharp import semmle.code.csharp.security.dataflow.flowsources.Remote - -/** - * A data flow source for unsafe Decompression extraction. - */ -abstract class DecompressionSource extends DataFlow::Node { } - -/** - * A data flow sink for unsafe zip extraction. - */ -abstract class DecompressionSink extends DataFlow::Node { } - -class ZipFile extends DecompressionSource { - ZipFile() { - exists(MethodCall mc | - ( - mc.getTarget().hasQualifiedName("System.IO.Compression", "ZipFile", "Open") and - this.asExpr() = mc.getArgument(0) and - mc.getArgument(1) = - any(EnumConstant e | - e.hasQualifiedName("System.IO.Compression", "ZipArchiveMode", ["Update", "Read"]) - ).getAnAccess() - or - mc.getTarget().hasQualifiedName("System.IO.Compression", "ZipFile", "OpenRead") and - this.asExpr() = mc.getArgument(0) - ) and - not mc.getArgument(0).getType().isConst() - ) - } -} - -/** A path argument to a call to the `ZipArchive` constructor call. */ -class ZipArchive extends DecompressionSource { - ZipArchive() { - exists(Constructor c | - c.getDeclaringType().hasQualifiedName("System.IO.Compression", "ZipArchive") - | - this.asExpr() = c.getACall() and - not c.getACall().getArgument(0).getType().isConst() and - c.getACall().getArgument(1) = - any(EnumConstant e | - e.hasQualifiedName("System.IO.Compression", "ZipArchiveMode", ["Update", "Read"]) - ).getAnAccess() - ) - } -} - -/** A Qualifier of the `Open()` method. */ -class ZipArchiveEntry extends DecompressionSink { - ZipArchiveEntry() { - exists(MethodCall mc | - mc.getTarget().hasQualifiedName("System.IO.Compression", "ZipArchiveEntry", "Open") and - this.asExpr() = mc.getQualifier() - ) - } -} - -/** A Caller of the `ExtractToFile` method. */ -class ZipFileExtensionsCallSink extends DecompressionSink { - ZipFileExtensionsCallSink() { - exists(MethodCall mc | - mc.getTarget().hasQualifiedName("System.IO.Compression", "ZipFileExtensions", "ExtractToFile") and - this.asExpr() = mc.getArgumentForName("source") and - mc.getArgumentForName("source") - .getType() - .hasQualifiedName("System.IO.Compression", "ZipArchiveEntry") - or - mc.getTarget() - .hasQualifiedName("System.IO.Compression", "ZipFileExtensions", "ExtractToDirectory") and - this.asExpr() = mc.getArgumentForName("source") and - mc.getArgumentForName("source") - .getType() - .hasQualifiedName("System.IO.Compression", "ZipArchive") - ) - } -} - -/** A Call to the `GZipStream` first arugument of Constructor Call . */ -class GZipStreamSink extends DecompressionSink, DecompressionSource { - GZipStreamSink() { - exists(Constructor mc | - mc.getDeclaringType().hasQualifiedName("System.IO.Compression", "GZipStream") and - this.asExpr() = mc.getACall().getArgument(0) and - mc.getACall().getArgument(1) = - any(EnumConstant e | - e.hasQualifiedName("System.IO.Compression", "CompressionMode", "Decompress") - ).getAnAccess() - ) - } -} - -/** A Call to the `BrotliStream` first arugument of Constructor Call . */ -class BrotliStreamSink extends DecompressionSink, DecompressionSource { - BrotliStreamSink() { - exists(Constructor mc | - mc.getDeclaringType().hasQualifiedName("System.IO.Compression", "BrotliStream") and - this.asExpr() = mc.getACall().getArgument(0) and - mc.getACall().getArgument(1) = - any(EnumConstant e | - e.hasQualifiedName("System.IO.Compression", "CompressionMode", "Decompress") - ).getAnAccess() - ) - } -} - -/** A Call to the `BrotliStream` first arugument of Constructor Call . */ -class DeflateStreamSink extends DecompressionSink, DecompressionSource { - DeflateStreamSink() { - exists(Constructor mc | - mc.getDeclaringType().hasQualifiedName("System.IO.Compression", "DeflateStream") and - this.asExpr() = mc.getACall().getArgument(0) and - mc.getACall().getArgument(1) = - any(EnumConstant e | - e.hasQualifiedName("System.IO.Compression", "CompressionMode", "Decompress") - ).getAnAccess() - ) - } -} - -/** A Call to the `ZLibStream` first arugument of Constructor Call . */ -class ZLibStreamSink extends DecompressionSink, DecompressionSource { - ZLibStreamSink() { - exists(Constructor mc | - mc.getDeclaringType().hasQualifiedName("System.IO.Compression", "ZLibStream") and - this.asExpr() = mc.getACall().getArgument(0) and - mc.getACall().getArgument(1) = - any(EnumConstant e | - e.hasQualifiedName("System.IO.Compression", "CompressionMode", "Decompress") - ).getAnAccess() - ) - } -} - -class SharpZipLibType extends RefType { - SharpZipLibType() { - this.hasQualifiedName("ICSharpCode.SharpZipLib.Zip", "ZipInputStream") or - this.hasQualifiedName("ICSharpCode.SharpZipLib.BZip2", "BZip2InputStream") or - this.hasQualifiedName("ICSharpCode.SharpZipLib.GZip", "GZipInputStream") or - this.hasQualifiedName("ICSharpCode.SharpZipLib.Lzw", "LzwInputStream") or - this.hasQualifiedName("ICSharpCode.SharpZipLib.Tar", "TarInputStream") or - this.hasQualifiedName("ICSharpCode.SharpZipLib.Zip.Compression.Streams", "InflaterInputStream") - } -} - -class SharpZipLibSource extends DecompressionSource { - SharpZipLibSource() { - exists(Constructor c | - c.getDeclaringType() instanceof SharpZipLibType and - this.asExpr() = c.getACall() and - not c.getDeclaringType().hasQualifiedName("ICSharpCode.SharpZipLib.Tar", "TarInputStream") - ) - } -} - -class SharpZipLibDecompressionMethod extends MethodCall { - SharpZipLibDecompressionMethod() { - exists(string methodName | - methodName = - [ - "Read", "ReadAsync", "ReadAtLeast", "CopyToAsync", "CopyTo", "CopyEntryContentsAsync", - "CopyEntryContents" - ] - | - this.getTarget().hasQualifiedName("ICSharpCode.SharpZipLib.Zip", "ZipInputStream", methodName) or - this.getTarget() - .hasQualifiedName("ICSharpCode.SharpZipLib.BZip2", "BZip2InputStream", methodName) or - this.getTarget() - .hasQualifiedName("ICSharpCode.SharpZipLib.GZip", "GZipInputStream", methodName) or - this.getTarget().hasQualifiedName("ICSharpCode.SharpZipLib.Lzw", "LzwInputStream", methodName) or - this.getTarget().hasQualifiedName("ICSharpCode.SharpZipLib.Tar", "TarInputStream", methodName) or - this.getTarget() - .hasQualifiedName("ICSharpCode.SharpZipLib.Zip.Compression.Streams", - "InflaterInputStream", methodName) or - this.getTarget().hasQualifiedName("System.IO", "Stream", methodName) - ) - } -} - -class K4osLZ4Source extends DecompressionSource { - K4osLZ4Source() { exists(K4osLZ4InitializeMethod ma | this.asExpr() = ma.getArgument(0)) } -} - -class K4osLZ4InitializeMethod extends MethodCall { - K4osLZ4InitializeMethod() { - this.getTarget().hasQualifiedName("K4os.Compression.LZ4.Streams", "LZ4Stream", "Decode") - } -} - -class K4osLZ4DecompressionMethod extends MethodCall { - K4osLZ4DecompressionMethod() { - exists(string methodName | methodName = ["Read", "ReadAsync", "CopyToAsync", "CopyTo"] | - this.getTarget() - .hasQualifiedName("K4os.Compression.LZ4.Streams", "LZ4DecoderStream", methodName) - ) - } -} - -class K4osLZ4LibSink extends DecompressionSink { - K4osLZ4LibSink() { exists(K4osLZ4DecompressionMethod mc | this.asExpr() = mc.getArgument(0)) } -} - -class SharpZipLibSink extends DecompressionSink { - SharpZipLibSink() { - exists(SharpZipLibDecompressionMethod mc | this.asExpr() = mc.getArgument(0)) - } -} - -class SharpZipLibFastZipSink extends DecompressionSink { - SharpZipLibFastZipSink() { - exists(MethodCall mc | - mc.getTarget().hasQualifiedName("ICSharpCode.SharpZipLib.Zip", "ZipInputStream", "ExtractZip") and - this.asExpr() = mc.getArgument(0) - ) - } -} +import experimental.dataflow.security.DecompressionBomb +import RemoteFlowSource /** * A taint tracking configuration for Decompression Bomb. */ -private module DecompressionBombConfig implements DataFlow::ConfigSig { - predicate isSource(DataFlow::Node source) { - source instanceof DecompressionSource - or - source instanceof RemoteFlowSource - } +module DecompressionBombModule implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource } - predicate isSink(DataFlow::Node sink) { sink instanceof DecompressionSink } + predicate isSink(DataFlow::Node sink) { sink instanceof DecompressionBomb::Sink } predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { - // var node2 = node1.ExtractToFile("./output.txt", true) - exists(MethodCall mc | - mc.getTarget().hasQualifiedName("System.IO.Compression", "ZipFileExtensions", "ExtractToFile") and - node2.asExpr() = mc and - node1.asExpr() = mc.getArgumentForName("source") - ) - or - exists(MethodCall mc | - mc.getTarget().hasQualifiedName("System.IO.Compression", "ZipArchive", "GetEntry") and - node1.asExpr() = mc.getQualifier() and - node2.asExpr() = mc - ) - or - // var node2 = node1.OpenReadStream() - exists(MethodCall mc | - mc.getTarget().hasQualifiedName("Microsoft.AspNetCore.Http", "IFormFile", "OpenReadStream") and - node2.asExpr() = mc and - node1.asExpr() = mc.getQualifier() - ) - or - exists(MethodCall mc, string methodName | - methodName = - [ - "Read", "ReadAsync", "ReadAtLeast", "CopyToAsync", "CopyTo", "CopyEntryContentsAsync", - "CopyEntryContents" - ] - | - ( - mc.getTarget().getDeclaringType() instanceof SharpZipLibType or - mc.getTarget().hasQualifiedName("System.IO", "Stream", methodName) - ) and - node1.asExpr() = mc.getQualifier() and - node2.asExpr() = mc.getArgument(0) - ) - or - exists(Constructor c | - c.getDeclaringType() instanceof SharpZipLibType and - node1.asExpr() = c.getACall().getArgument(0) and - node2.asExpr() = c.getACall() - ) - or - exists(K4osLZ4DecompressionMethod mc | - node1.asExpr() = mc.getQualifier() and - node2.asExpr() = mc.getArgument(0) - ) - or - exists(K4osLZ4InitializeMethod mc | - node2.asExpr() = mc and - node1.asExpr() = mc.getArgument(0) - ) + any(DecompressionBomb::AdditionalStep a).isAdditionalFlowStep(node1, node2) } } -module DecompressionBomb = TaintTracking::Global; +module DecompressionBombConfig = TaintTracking::Global; -import DecompressionBomb::PathGraph +import DecompressionBombConfig::PathGraph -from DecompressionBomb::PathNode source, DecompressionBomb::PathNode sink -where DecompressionBomb::flowPath(source, sink) +from DecompressionBombConfig::PathNode source, DecompressionBombConfig::PathNode sink +where DecompressionBombConfig::flowPath(source, sink) select sink.getNode(), source, sink, "This uncontrolled depends on a $@.", source.getNode(), "this" diff --git a/csharp/ql/src/experimental/CWE-502-DecompressionBombs/RemoteFlowSource.qll b/csharp/ql/src/experimental/CWE-502-DecompressionBombs/RemoteFlowSource.qll index 337f3921adc6..6d80976e25fe 100644 --- a/csharp/ql/src/experimental/CWE-502-DecompressionBombs/RemoteFlowSource.qll +++ b/csharp/ql/src/experimental/CWE-502-DecompressionBombs/RemoteFlowSource.qll @@ -1,9 +1,10 @@ import csharp import semmle.code.csharp.security.dataflow.flowsources.Remote -/** A data flow source of remote user input by Form File (ASP.NET unvalidated request data). */ +/** A data flow source of remote user input by Form File (ASP.NET core request data). */ class FormFile extends AspNetRemoteFlowSource { FormFile() { + // openReadStream is already implemented but I'm putting this here because of having a uniform class for FormFile exists(MethodCall mc | mc.getTarget().hasQualifiedName("Microsoft.AspNetCore.Http", "IFormFile", "OpenReadStream") and this.asExpr() = mc @@ -22,12 +23,10 @@ class FormFile extends AspNetRemoteFlowSource { ) } - override string getSourceType() { - result = "ASP.NET unvalidated request data from multipart request" - } + override string getSourceType() { result = "ASP.NET core request data from multipart request" } } -/** A data flow source of remote user input by Form (ASP.NET unvalidated request data). */ +/** A data flow source of remote user input by Form (ASP.NET core request data). */ class FormCollection extends AspNetRemoteFlowSource { FormCollection() { exists(Property fa | @@ -35,14 +34,22 @@ class FormCollection extends AspNetRemoteFlowSource { fa.hasName("Keys") and this.asExpr() = fa.getAnAccess() ) + or + exists(Call c | + c.getTarget() + .getDeclaringType() + .hasQualifiedName("Microsoft.AspNetCore.Http", "IFormCollection") and + c.getTarget().getName() = "TryGetValue" and + this.asDefinition().getTargetAccess() = c.getArgument(1) + ) } override string getSourceType() { - result = "ASP.NET unvalidated request data from multipart request Form Keys" + result = "ASP.NET core request data from multipart request Form Keys" } } -/** A data flow source of remote user input by Headers (ASP.NET unvalidated request data). */ +/** A data flow source of remote user input by Headers (ASP.NET core request data). */ class HeaderDictionary extends AspNetRemoteFlowSource { HeaderDictionary() { exists(Property fa | @@ -51,7 +58,5 @@ class HeaderDictionary extends AspNetRemoteFlowSource { ) } - override string getSourceType() { - result = "ASP.NET unvalidated request data from Headers of request" - } + override string getSourceType() { result = "ASP.NET core request data from Headers of request" } } diff --git a/csharp/ql/src/experimental/dataflow/security/DecompressionBomb.qll b/csharp/ql/src/experimental/dataflow/security/DecompressionBomb.qll new file mode 100644 index 000000000000..f09906cc5a8f --- /dev/null +++ b/csharp/ql/src/experimental/dataflow/security/DecompressionBomb.qll @@ -0,0 +1,273 @@ +import csharp + +module DecompressionBomb { + /** + * A data flow sink for unsafe zip extraction. + */ + abstract class Sink extends DataFlow::Node { } + + /** + * A data flow Additional step for unsafe zip extraction. + */ + class AdditionalStep extends string { + AdditionalStep() { this = "DecompressionBomb AdditionalStep" } + + abstract predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2); + } +} + +module SystemIoCompression { + /** A `Open` method of ZipFile. */ + class ZipArchiveOpen extends MethodCall { + ZipArchiveOpen() { + exists(MethodCall c | + c.getTarget().hasQualifiedName("System.IO.Compression", "ZipFile", "Open") and + this = c + ) + } + } + + /** A Call to `Open` method considered as Decompression Bomb sink. */ + class ZipArchiveEntryOpen extends DecompressionBomb::Sink { + ZipArchiveEntryOpen() { + exists(MethodCall c | + c.getTarget().hasQualifiedName("System.IO.Compression", "ZipArchiveEntry", "Open") and + this.asExpr() = c + ) + } + } + + /** A Caller of the `ExtractToFile` method. */ + class ZipFileExtensionsCallSink extends DecompressionBomb::Sink { + ZipFileExtensionsCallSink() { + exists(MethodCall mc | + mc.getTarget() + .hasQualifiedName("System.IO.Compression", "ZipFileExtensions", "ExtractToFile") and + this.asExpr() = mc.getArgumentForName("source") and + mc.getArgumentForName("source") + .getType() + .hasQualifiedName("System.IO.Compression", "ZipArchiveEntry") + or + mc.getTarget() + .hasQualifiedName("System.IO.Compression", "ZipFileExtensions", "ExtractToDirectory") and + this.asExpr() = mc.getArgumentForName("source") and + mc.getArgumentForName("source") + .getType() + .hasQualifiedName("System.IO.Compression", "ZipArchive") + ) + } + } + + /** A Call to the `GZipStream` first argument of Constructor Call . */ + class GZipStreamSink extends DecompressionBomb::Sink { + GZipStreamSink() { + exists(Constructor mc | + mc.getDeclaringType().hasQualifiedName("System.IO.Compression", "GZipStream") and + this.asExpr() = mc.getACall().getArgument(0) and + mc.getACall().getArgument(1) = + any(EnumConstant e | + e.hasQualifiedName("System.IO.Compression", "CompressionMode", "Decompress") + ).getAnAccess() + ) + } + } + + /** A Call to the `BrotliStream` first argument of Constructor Call . */ + class BrotliStreamSink extends DecompressionBomb::Sink { + BrotliStreamSink() { + exists(Constructor mc | + mc.getDeclaringType().hasQualifiedName("System.IO.Compression", "BrotliStream") and + this.asExpr() = mc.getACall().getArgument(0) and + mc.getACall().getArgument(1) = + any(EnumConstant e | + e.hasQualifiedName("System.IO.Compression", "CompressionMode", "Decompress") + ).getAnAccess() + ) + } + } + + /** A Call to the `BrotliStream` first argument of Constructor Call . */ + class DeflateStreamSink extends DecompressionBomb::Sink { + DeflateStreamSink() { + exists(Constructor mc | + mc.getDeclaringType().hasQualifiedName("System.IO.Compression", "DeflateStream") and + this.asExpr() = mc.getACall().getArgument(0) and + mc.getACall().getArgument(1) = + any(EnumConstant e | + e.hasQualifiedName("System.IO.Compression", "CompressionMode", "Decompress") + ).getAnAccess() + ) + } + } + + /** A Call to the `ZLibStream` first argument of Constructor Call . */ + class ZLibStreamSink extends DecompressionBomb::Sink { + ZLibStreamSink() { + exists(Constructor mc | + mc.getDeclaringType().hasQualifiedName("System.IO.Compression", "ZLibStream") and + this.asExpr() = mc.getACall().getArgument(0) and + mc.getACall().getArgument(1) = + any(EnumConstant e | + e.hasQualifiedName("System.IO.Compression", "CompressionMode", "Decompress") + ).getAnAccess() + ) + } + } + + class DecompressionAdditionalStep extends DecompressionBomb::AdditionalStep { + override predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { + exists(MethodCall mc | + mc.getTarget() + .hasQualifiedName("System.IO.Compression", "ZipFileExtensions", "ExtractToFile") and + node2.asExpr() = mc and + node1.asExpr() = mc.getArgumentForName("source") + ) + or + exists(MethodCall mc | + mc.getTarget().hasQualifiedName("System.IO.Compression", "ZipArchive", "GetEntry") and + node1.asExpr() = mc.getQualifier() and + node2.asExpr() = mc + ) + or + exists(Call mc | + mc.getTarget().getDeclaringType().hasQualifiedName("System.IO.Compression", "ZipArchive") and + node1.asExpr() = mc.getArgument(0) and + node2.asExpr() = mc + ) + or + exists(ZipArchiveOpen zo | + node1.asExpr() = zo.getArgument(0) and + node2.asExpr() = zo + ) + or + exists(MethodCall mc | + mc.getTarget().hasQualifiedName("System.IO.Compression", "ZipArchiveEntry", "Open") + | + node1.asExpr() = mc.getQualifier() and + node2.asExpr() = mc + ) + } + } +} + +module K4osLz4 { + /** A Decode MethodCall of LZ4Stream */ + class K4osLZ4InitializeMethod extends MethodCall { + K4osLZ4InitializeMethod() { + this.getTarget().hasQualifiedName("K4os.Compression.LZ4.Streams", "LZ4Stream", "Decode") + } + } + + /** The Methods responsible for reading Decompressed data */ + class K4osLZ4DecompressionMethod extends MethodCall { + K4osLZ4DecompressionMethod() { + exists(string methodName | methodName = ["Read", "ReadAsync", "CopyToAsync", "CopyTo"] | + this.getTarget() + .hasQualifiedName("K4os.Compression.LZ4.Streams", "LZ4DecoderStream", methodName) + ) + } + } + + /** The first argument of LZ4DecoderStream decompression methods as decompression bomb sink */ + class K4osLZ4LibSink extends DecompressionBomb::Sink { + K4osLZ4LibSink() { exists(K4osLZ4DecompressionMethod mc | this.asExpr() = mc.getArgument(0)) } + } + + class DecompressionAdditionalStep extends DecompressionBomb::AdditionalStep { + override predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { + exists(K4osLZ4DecompressionMethod mc | + node1.asExpr() = mc.getQualifier() and + node2.asExpr() = mc.getArgument(0) + ) + or + exists(K4osLZ4InitializeMethod mc | + node2.asExpr() = mc and + node1.asExpr() = mc.getArgument(0) + ) + } + } +} + +/** The first argument of LZ4DecoderStream decompression methods as decompression bomb sink */ +module SharpZip { + class SharpZipLibType extends RefType { + SharpZipLibType() { + this.hasQualifiedName("ICSharpCode.SharpZipLib.Zip", "ZipInputStream") or + this.hasQualifiedName("ICSharpCode.SharpZipLib.BZip2", "BZip2InputStream") or + this.hasQualifiedName("ICSharpCode.SharpZipLib.GZip", "GZipInputStream") or + this.hasQualifiedName("ICSharpCode.SharpZipLib.Lzw", "LzwInputStream") or + this.hasQualifiedName("ICSharpCode.SharpZipLib.Tar", "TarInputStream") or + this.hasQualifiedName("ICSharpCode.SharpZipLib.Zip.Compression.Streams", "InflaterInputStream") + } + } + + /** The methods for decompressing data */ + class SharpZipLibDecompressionMethod extends MethodCall { + SharpZipLibDecompressionMethod() { + exists(string methodName | + methodName = + [ + "Read", "ReadAsync", "ReadAtLeast", "CopyToAsync", "CopyTo", "CopyEntryContentsAsync", + "CopyEntryContents" + ] + | + this.getTarget() + .hasQualifiedName("ICSharpCode.SharpZipLib.Zip", "ZipInputStream", methodName) or + this.getTarget() + .hasQualifiedName("ICSharpCode.SharpZipLib.BZip2", "BZip2InputStream", methodName) or + this.getTarget() + .hasQualifiedName("ICSharpCode.SharpZipLib.GZip", "GZipInputStream", methodName) or + this.getTarget() + .hasQualifiedName("ICSharpCode.SharpZipLib.Lzw", "LzwInputStream", methodName) or + this.getTarget() + .hasQualifiedName("ICSharpCode.SharpZipLib.Tar", "TarInputStream", methodName) or + this.getTarget() + .hasQualifiedName("ICSharpCode.SharpZipLib.Zip.Compression.Streams", + "InflaterInputStream", methodName) + ) + } + } + + /** The first argument of SharpZipLib decompression methods as decompression bomb sink */ + class SharpZipLibSink extends DecompressionBomb::Sink { + SharpZipLibSink() { + exists(SharpZipLibDecompressionMethod mc | this.asExpr() = mc.getArgument(0)) + } + } + + /** The `ExtractZip` first argument or method call considered as decompression sink bomb */ + class SharpZipLibFastZipSink extends DecompressionBomb::Sink { + SharpZipLibFastZipSink() { + exists(MethodCall mc | + mc.getTarget() + .hasQualifiedName("ICSharpCode.SharpZipLib.Zip", "ZipInputStream", "ExtractZip") and + this.asExpr() = mc.getArgument(0) + ) + } + } + + class DecompressionAdditionalStep extends DecompressionBomb::AdditionalStep { + override predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { + exists(MethodCall mc, string methodName | + methodName = + [ + "Read", "ReadAsync", "ReadAtLeast", "CopyToAsync", "CopyTo", "CopyEntryContentsAsync", + "CopyEntryContents" + ] + | + ( + mc.getTarget().getDeclaringType() instanceof SharpZipLibType or + mc.getTarget().hasQualifiedName("System.IO", "Stream", methodName) + ) and + node1.asExpr() = mc.getQualifier() and + node2.asExpr() = mc.getArgument(0) + ) + or + exists(Constructor c | + c.getDeclaringType() instanceof SharpZipLibType and + node1.asExpr() = c.getACall().getArgument(0) and + node2.asExpr() = c.getACall() + ) + } + } +} diff --git a/csharp/ql/test/experimental/CWE-502-DecompressionBombs/DecompressionBombs.cs b/csharp/ql/test/experimental/CWE-502-DecompressionBombs/DecompressionBombs.cs index 8dbd665d7866..94ef64b717f0 100644 --- a/csharp/ql/test/experimental/CWE-502-DecompressionBombs/DecompressionBombs.cs +++ b/csharp/ql/test/experimental/CWE-502-DecompressionBombs/DecompressionBombs.cs @@ -1,38 +1,41 @@ using System.IO.Compression; using Microsoft.AspNetCore.Mvc; +using Microsoft.Extensions.Primitives; +namespace MultipartFormWebAPITest.Controllers; -namespace MultipartFormWebAPITest.Controllers +[Route("api/[controller]")] +[ApiController] +public class ZipFile1Controller : ControllerBase { - [Route("api/[controller]")] - [ApiController] - public class ZipFile1Controller : ControllerBase + [HttpPost] + public IActionResult Post(List files) { - // POST api/ - [HttpPost] - public string Post(List files) + foreach (var formFile in files) { - if (!Request.ContentType!.StartsWith("multipart/form-data")) - return "400"; - if (files.Count == 0) - return "400"; - foreach (var formFile in files) - { - using var readStream = formFile.OpenReadStream(); - if (readStream.Length == 0) return "400"; - ZipHelpers.Bomb3(readStream); - ZipHelpers.Bomb2(formFile.FileName); - ZipHelpers.BombSafe(formFile.FileName); - } - var tmp = Request.Form["aa"]; - var tmp2 = Request.Form.Keys; - // when we don't have only one file as body - ZipHelpers.Bomb3(Request.Body); - ZipHelpers.Bomb2(Request.Query["param1"].ToString()); - var headers = Request.Headers; - ZipHelpers.BombSafe(headers.ETag); - return "200"; + ZipHelpers.Bomb2(formFile.FileName); + ZipHelpers.BombSafe(formFile.FileName); + ZipHelpers.Bomb3(formFile.OpenReadStream()); } + + + StringValues file1; + StringValues header1; + Request.Form.TryGetValue("file", out file1); + Request.Headers.TryGetValue("Header1", out header1); + Request.Query.TryGetValue("queryParam1", out var qParam); + + var a = Request.Headers["a"]; + var b = Request.Headers.ContentType; + + var d = Array.Empty>(); + Request.Headers.CopyTo(d, 0); + + ZipHelpers.Bomb2(file1!); + ZipHelpers.Bomb2(header1!); + ZipHelpers.Bomb2(qParam!); + + return Accepted(); } } @@ -43,22 +46,28 @@ public static void Bomb3(Stream compressedFileStream) using var decompressor = new GZipStream(compressedFileStream, CompressionMode.Decompress); using var ms = new MemoryStream(); decompressor.CopyTo(ms); + using var decompressor2 = new BrotliStream(compressedFileStream, CompressionMode.Decompress); + using var ms2 = new MemoryStream(); + decompressor2.CopyTo(ms2); + using var decompressor3 = new DeflateStream(compressedFileStream, CompressionMode.Decompress); + using var ms3 = new MemoryStream(); + decompressor3.CopyTo(ms3); } public static void Bomb2(string filename) { using var zipToOpen = new FileStream(filename, FileMode.Open); using var archive = new ZipArchive(zipToOpen, ZipArchiveMode.Read); + archive.ExtractToDirectory("./tmp/", true); + (archive.GetEntry("aaa") ?? throw new InvalidOperationException()).ExtractToFile("/tmp/tmp"); foreach (var entry in archive.Entries) entry.ExtractToFile("./output.txt", true); // Sensitive } public static void BombSafe(string filename) { const long maxLength = 10 * 1024 * 1024; // 10MB - using var zipFile = ZipFile.OpenRead(filename); - // Quickly check the value from the zip header - // we don't worry about malicious zip headers anymore - // because we read stream until the value of header not more + using var zipFile = ZipFile.Open(filename, ZipArchiveMode.Read); + // Quickly check the value from the zip header var declaredSize = zipFile.Entries.Sum(entry => entry.Length); if (declaredSize > maxLength) throw new Exception("Archive is too big"); @@ -71,6 +80,11 @@ public static void BombSafe(string filename) using var ms = new MemoryStream(); maxLengthStream.CopyTo(ms); } + + const string zipPath = @"c:\users\exampleuser\start.zip"; + const string extractPath = @"c:\users\exampleuser\extract"; + using var archive = ZipFile.Open(zipPath, ZipArchiveMode.Update); + archive.ExtractToDirectory(extractPath); } } @@ -108,7 +122,6 @@ public override int Read(byte[] buffer, int offset, int count) return result; } - // TODO ReadAsync public override void Flush() { From 825a2dd566e99e3405967a6ba78194ec0d6440cf Mon Sep 17 00:00:00 2001 From: amammad <77095239+amammad@users.noreply.github.com> Date: Fri, 9 Feb 2024 12:33:45 +0400 Subject: [PATCH 11/11] hasQualifiedName to hasFullyQualifiedName --- .../RemoteFlowSource.qll | 12 ++-- .../dataflow/security/DecompressionBomb.qll | 72 ++++++++++--------- 2 files changed, 44 insertions(+), 40 deletions(-) diff --git a/csharp/ql/src/experimental/CWE-502-DecompressionBombs/RemoteFlowSource.qll b/csharp/ql/src/experimental/CWE-502-DecompressionBombs/RemoteFlowSource.qll index 6d80976e25fe..470ac4ee9ae5 100644 --- a/csharp/ql/src/experimental/CWE-502-DecompressionBombs/RemoteFlowSource.qll +++ b/csharp/ql/src/experimental/CWE-502-DecompressionBombs/RemoteFlowSource.qll @@ -6,18 +6,18 @@ class FormFile extends AspNetRemoteFlowSource { FormFile() { // openReadStream is already implemented but I'm putting this here because of having a uniform class for FormFile exists(MethodCall mc | - mc.getTarget().hasQualifiedName("Microsoft.AspNetCore.Http", "IFormFile", "OpenReadStream") and + mc.getTarget().hasFullyQualifiedName("Microsoft.AspNetCore.Http", "IFormFile", "OpenReadStream") and this.asExpr() = mc ) or exists(MethodCall mc | mc.getTarget() - .hasQualifiedName("Microsoft.AspNetCore.Http", "IFormFile", ["CopyTo", "CopyToAsync"]) and + .hasFullyQualifiedName("Microsoft.AspNetCore.Http", "IFormFile", ["CopyTo", "CopyToAsync"]) and this.asParameter() = mc.getTarget().getParameter(0) ) or exists(Property fa | - fa.getDeclaringType().hasQualifiedName("Microsoft.AspNetCore.Http", "IFormFile") and + fa.getDeclaringType().hasFullyQualifiedName("Microsoft.AspNetCore.Http", "IFormFile") and fa.hasName(["ContentType", "ContentDisposition", "Name", "FileName"]) and this.asExpr() = fa.getAnAccess() ) @@ -30,7 +30,7 @@ class FormFile extends AspNetRemoteFlowSource { class FormCollection extends AspNetRemoteFlowSource { FormCollection() { exists(Property fa | - fa.getDeclaringType().hasQualifiedName("Microsoft.AspNetCore.Http", "IFormCollection") and + fa.getDeclaringType().hasFullyQualifiedName("Microsoft.AspNetCore.Http", "IFormCollection") and fa.hasName("Keys") and this.asExpr() = fa.getAnAccess() ) @@ -38,7 +38,7 @@ class FormCollection extends AspNetRemoteFlowSource { exists(Call c | c.getTarget() .getDeclaringType() - .hasQualifiedName("Microsoft.AspNetCore.Http", "IFormCollection") and + .hasFullyQualifiedName("Microsoft.AspNetCore.Http", "IFormCollection") and c.getTarget().getName() = "TryGetValue" and this.asDefinition().getTargetAccess() = c.getArgument(1) ) @@ -53,7 +53,7 @@ class FormCollection extends AspNetRemoteFlowSource { class HeaderDictionary extends AspNetRemoteFlowSource { HeaderDictionary() { exists(Property fa | - fa.getDeclaringType().hasQualifiedName("Microsoft.AspNetCore.Http", "IHeaderDictionary") and + fa.getDeclaringType().hasFullyQualifiedName("Microsoft.AspNetCore.Http", "IHeaderDictionary") and this.asExpr() = fa.getAnAccess() ) } diff --git a/csharp/ql/src/experimental/dataflow/security/DecompressionBomb.qll b/csharp/ql/src/experimental/dataflow/security/DecompressionBomb.qll index f09906cc5a8f..803c0c9154ec 100644 --- a/csharp/ql/src/experimental/dataflow/security/DecompressionBomb.qll +++ b/csharp/ql/src/experimental/dataflow/security/DecompressionBomb.qll @@ -21,7 +21,7 @@ module SystemIoCompression { class ZipArchiveOpen extends MethodCall { ZipArchiveOpen() { exists(MethodCall c | - c.getTarget().hasQualifiedName("System.IO.Compression", "ZipFile", "Open") and + c.getTarget().hasFullyQualifiedName("System.IO.Compression", "ZipFile", "Open") and this = c ) } @@ -31,7 +31,7 @@ module SystemIoCompression { class ZipArchiveEntryOpen extends DecompressionBomb::Sink { ZipArchiveEntryOpen() { exists(MethodCall c | - c.getTarget().hasQualifiedName("System.IO.Compression", "ZipArchiveEntry", "Open") and + c.getTarget().hasFullyQualifiedName("System.IO.Compression", "ZipArchiveEntry", "Open") and this.asExpr() = c ) } @@ -42,18 +42,19 @@ module SystemIoCompression { ZipFileExtensionsCallSink() { exists(MethodCall mc | mc.getTarget() - .hasQualifiedName("System.IO.Compression", "ZipFileExtensions", "ExtractToFile") and + .hasFullyQualifiedName("System.IO.Compression", "ZipFileExtensions", "ExtractToFile") and this.asExpr() = mc.getArgumentForName("source") and mc.getArgumentForName("source") .getType() - .hasQualifiedName("System.IO.Compression", "ZipArchiveEntry") + .hasFullyQualifiedName("System.IO.Compression", "ZipArchiveEntry") or mc.getTarget() - .hasQualifiedName("System.IO.Compression", "ZipFileExtensions", "ExtractToDirectory") and + .hasFullyQualifiedName("System.IO.Compression", "ZipFileExtensions", + "ExtractToDirectory") and this.asExpr() = mc.getArgumentForName("source") and mc.getArgumentForName("source") .getType() - .hasQualifiedName("System.IO.Compression", "ZipArchive") + .hasFullyQualifiedName("System.IO.Compression", "ZipArchive") ) } } @@ -62,11 +63,11 @@ module SystemIoCompression { class GZipStreamSink extends DecompressionBomb::Sink { GZipStreamSink() { exists(Constructor mc | - mc.getDeclaringType().hasQualifiedName("System.IO.Compression", "GZipStream") and + mc.getDeclaringType().hasFullyQualifiedName("System.IO.Compression", "GZipStream") and this.asExpr() = mc.getACall().getArgument(0) and mc.getACall().getArgument(1) = any(EnumConstant e | - e.hasQualifiedName("System.IO.Compression", "CompressionMode", "Decompress") + e.hasFullyQualifiedName("System.IO.Compression", "CompressionMode", "Decompress") ).getAnAccess() ) } @@ -76,11 +77,11 @@ module SystemIoCompression { class BrotliStreamSink extends DecompressionBomb::Sink { BrotliStreamSink() { exists(Constructor mc | - mc.getDeclaringType().hasQualifiedName("System.IO.Compression", "BrotliStream") and + mc.getDeclaringType().hasFullyQualifiedName("System.IO.Compression", "BrotliStream") and this.asExpr() = mc.getACall().getArgument(0) and mc.getACall().getArgument(1) = any(EnumConstant e | - e.hasQualifiedName("System.IO.Compression", "CompressionMode", "Decompress") + e.hasFullyQualifiedName("System.IO.Compression", "CompressionMode", "Decompress") ).getAnAccess() ) } @@ -90,11 +91,11 @@ module SystemIoCompression { class DeflateStreamSink extends DecompressionBomb::Sink { DeflateStreamSink() { exists(Constructor mc | - mc.getDeclaringType().hasQualifiedName("System.IO.Compression", "DeflateStream") and + mc.getDeclaringType().hasFullyQualifiedName("System.IO.Compression", "DeflateStream") and this.asExpr() = mc.getACall().getArgument(0) and mc.getACall().getArgument(1) = any(EnumConstant e | - e.hasQualifiedName("System.IO.Compression", "CompressionMode", "Decompress") + e.hasFullyQualifiedName("System.IO.Compression", "CompressionMode", "Decompress") ).getAnAccess() ) } @@ -104,11 +105,11 @@ module SystemIoCompression { class ZLibStreamSink extends DecompressionBomb::Sink { ZLibStreamSink() { exists(Constructor mc | - mc.getDeclaringType().hasQualifiedName("System.IO.Compression", "ZLibStream") and + mc.getDeclaringType().hasFullyQualifiedName("System.IO.Compression", "ZLibStream") and this.asExpr() = mc.getACall().getArgument(0) and mc.getACall().getArgument(1) = any(EnumConstant e | - e.hasQualifiedName("System.IO.Compression", "CompressionMode", "Decompress") + e.hasFullyQualifiedName("System.IO.Compression", "CompressionMode", "Decompress") ).getAnAccess() ) } @@ -118,19 +119,21 @@ module SystemIoCompression { override predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { exists(MethodCall mc | mc.getTarget() - .hasQualifiedName("System.IO.Compression", "ZipFileExtensions", "ExtractToFile") and + .hasFullyQualifiedName("System.IO.Compression", "ZipFileExtensions", "ExtractToFile") and node2.asExpr() = mc and node1.asExpr() = mc.getArgumentForName("source") ) or exists(MethodCall mc | - mc.getTarget().hasQualifiedName("System.IO.Compression", "ZipArchive", "GetEntry") and + mc.getTarget().hasFullyQualifiedName("System.IO.Compression", "ZipArchive", "GetEntry") and node1.asExpr() = mc.getQualifier() and node2.asExpr() = mc ) or exists(Call mc | - mc.getTarget().getDeclaringType().hasQualifiedName("System.IO.Compression", "ZipArchive") and + mc.getTarget() + .getDeclaringType() + .hasFullyQualifiedName("System.IO.Compression", "ZipArchive") and node1.asExpr() = mc.getArgument(0) and node2.asExpr() = mc ) @@ -141,7 +144,7 @@ module SystemIoCompression { ) or exists(MethodCall mc | - mc.getTarget().hasQualifiedName("System.IO.Compression", "ZipArchiveEntry", "Open") + mc.getTarget().hasFullyQualifiedName("System.IO.Compression", "ZipArchiveEntry", "Open") | node1.asExpr() = mc.getQualifier() and node2.asExpr() = mc @@ -154,7 +157,7 @@ module K4osLz4 { /** A Decode MethodCall of LZ4Stream */ class K4osLZ4InitializeMethod extends MethodCall { K4osLZ4InitializeMethod() { - this.getTarget().hasQualifiedName("K4os.Compression.LZ4.Streams", "LZ4Stream", "Decode") + this.getTarget().hasFullyQualifiedName("K4os.Compression.LZ4.Streams", "LZ4Stream", "Decode") } } @@ -163,7 +166,7 @@ module K4osLz4 { K4osLZ4DecompressionMethod() { exists(string methodName | methodName = ["Read", "ReadAsync", "CopyToAsync", "CopyTo"] | this.getTarget() - .hasQualifiedName("K4os.Compression.LZ4.Streams", "LZ4DecoderStream", methodName) + .hasFullyQualifiedName("K4os.Compression.LZ4.Streams", "LZ4DecoderStream", methodName) ) } } @@ -192,12 +195,13 @@ module K4osLz4 { module SharpZip { class SharpZipLibType extends RefType { SharpZipLibType() { - this.hasQualifiedName("ICSharpCode.SharpZipLib.Zip", "ZipInputStream") or - this.hasQualifiedName("ICSharpCode.SharpZipLib.BZip2", "BZip2InputStream") or - this.hasQualifiedName("ICSharpCode.SharpZipLib.GZip", "GZipInputStream") or - this.hasQualifiedName("ICSharpCode.SharpZipLib.Lzw", "LzwInputStream") or - this.hasQualifiedName("ICSharpCode.SharpZipLib.Tar", "TarInputStream") or - this.hasQualifiedName("ICSharpCode.SharpZipLib.Zip.Compression.Streams", "InflaterInputStream") + this.hasFullyQualifiedName("ICSharpCode.SharpZipLib.Zip", "ZipInputStream") or + this.hasFullyQualifiedName("ICSharpCode.SharpZipLib.BZip2", "BZip2InputStream") or + this.hasFullyQualifiedName("ICSharpCode.SharpZipLib.GZip", "GZipInputStream") or + this.hasFullyQualifiedName("ICSharpCode.SharpZipLib.Lzw", "LzwInputStream") or + this.hasFullyQualifiedName("ICSharpCode.SharpZipLib.Tar", "TarInputStream") or + this.hasFullyQualifiedName("ICSharpCode.SharpZipLib.Zip.Compression.Streams", + "InflaterInputStream") } } @@ -212,17 +216,17 @@ module SharpZip { ] | this.getTarget() - .hasQualifiedName("ICSharpCode.SharpZipLib.Zip", "ZipInputStream", methodName) or + .hasFullyQualifiedName("ICSharpCode.SharpZipLib.Zip", "ZipInputStream", methodName) or this.getTarget() - .hasQualifiedName("ICSharpCode.SharpZipLib.BZip2", "BZip2InputStream", methodName) or + .hasFullyQualifiedName("ICSharpCode.SharpZipLib.BZip2", "BZip2InputStream", methodName) or this.getTarget() - .hasQualifiedName("ICSharpCode.SharpZipLib.GZip", "GZipInputStream", methodName) or + .hasFullyQualifiedName("ICSharpCode.SharpZipLib.GZip", "GZipInputStream", methodName) or this.getTarget() - .hasQualifiedName("ICSharpCode.SharpZipLib.Lzw", "LzwInputStream", methodName) or + .hasFullyQualifiedName("ICSharpCode.SharpZipLib.Lzw", "LzwInputStream", methodName) or this.getTarget() - .hasQualifiedName("ICSharpCode.SharpZipLib.Tar", "TarInputStream", methodName) or + .hasFullyQualifiedName("ICSharpCode.SharpZipLib.Tar", "TarInputStream", methodName) or this.getTarget() - .hasQualifiedName("ICSharpCode.SharpZipLib.Zip.Compression.Streams", + .hasFullyQualifiedName("ICSharpCode.SharpZipLib.Zip.Compression.Streams", "InflaterInputStream", methodName) ) } @@ -240,7 +244,7 @@ module SharpZip { SharpZipLibFastZipSink() { exists(MethodCall mc | mc.getTarget() - .hasQualifiedName("ICSharpCode.SharpZipLib.Zip", "ZipInputStream", "ExtractZip") and + .hasFullyQualifiedName("ICSharpCode.SharpZipLib.Zip", "ZipInputStream", "ExtractZip") and this.asExpr() = mc.getArgument(0) ) } @@ -257,7 +261,7 @@ module SharpZip { | ( mc.getTarget().getDeclaringType() instanceof SharpZipLibType or - mc.getTarget().hasQualifiedName("System.IO", "Stream", methodName) + mc.getTarget().hasFullyQualifiedName("System.IO", "Stream", methodName) ) and node1.asExpr() = mc.getQualifier() and node2.asExpr() = mc.getArgument(0)