From 99d1cf70be9e53cac5eaef85d062093c745ca045 Mon Sep 17 00:00:00 2001 From: Luke Cartey Date: Mon, 20 Aug 2018 14:49:55 +0100 Subject: [PATCH] C#: ZipSlip - Update name, description and message. This commit updates the name, description and message to better match the house style for the security queries. --- csharp/ql/src/Security Features/CWE-022/ZipSlip.ql | 10 ++++++---- .../CWE-022/ZipSlip/ZipSlip.expected | 12 ++++++------ 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/csharp/ql/src/Security Features/CWE-022/ZipSlip.ql b/csharp/ql/src/Security Features/CWE-022/ZipSlip.ql index 9d4da37358f..e170c279815 100644 --- a/csharp/ql/src/Security Features/CWE-022/ZipSlip.ql +++ b/csharp/ql/src/Security Features/CWE-022/ZipSlip.ql @@ -1,7 +1,9 @@ /** - * @name Potential ZipSlip vulnerability - * @description When extracting files from an archive, don't add archive item's path to the target file system path. Archive path can be relative and can lead to - * file system access outside of the expected file system target path, leading to malicious config changes and remote code execution via lay-and-wait technique + * @name Arbitrary file write during zip extraction ("ZipSlip") + * @description Extracting files from a malicious zip archive without validating that the + * destination file path is within the destination directory can cause files outside + * the destination directory to be overwritten, due to the possible presence of + * directory traversal elements ("..") in archive paths. * @kind problem * @id cs/zipslip * @problem.severity error @@ -14,4 +16,4 @@ import semmle.code.csharp.security.dataflow.ZipSlip::ZipSlip from TaintTrackingConfiguration zipTaintTracking, DataFlow::Node source, DataFlow::Node sink where zipTaintTracking.hasFlow(source, sink) -select sink, "Make sure to sanitize relative archive item path before creating path for file extraction if the source of $@ is untrusted", source, "zip archive" \ No newline at end of file +select sink, "Unsanitized zip archive $@ which may contain '..' used in a file system operation.", source, "item path" \ No newline at end of file diff --git a/csharp/ql/test/query-tests/Security Features/CWE-022/ZipSlip/ZipSlip.expected b/csharp/ql/test/query-tests/Security Features/CWE-022/ZipSlip/ZipSlip.expected index 6ac906c7687..c8793a0671d 100644 --- a/csharp/ql/test/query-tests/Security Features/CWE-022/ZipSlip/ZipSlip.expected +++ b/csharp/ql/test/query-tests/Security Features/CWE-022/ZipSlip/ZipSlip.expected @@ -1,6 +1,6 @@ -| ZipSlip.cs:24:41:24:52 | access to local variable destFileName | Make sure to sanitize relative archive item path before creating path for file extraction if the source of $@ is untrusted | ZipSlip.cs:19:31:19:44 | access to property FullName | zip archive | -| ZipSlip.cs:32:41:32:52 | access to local variable destFilePath | Make sure to sanitize relative archive item path before creating path for file extraction if the source of $@ is untrusted | ZipSlip.cs:16:52:16:65 | access to property FullName | zip archive | -| ZipSlip.cs:61:74:61:85 | access to local variable destFilePath | Make sure to sanitize relative archive item path before creating path for file extraction if the source of $@ is untrusted | ZipSlip.cs:54:72:54:85 | access to property FullName | zip archive | -| ZipSlip.cs:68:71:68:82 | access to local variable destFilePath | Make sure to sanitize relative archive item path before creating path for file extraction if the source of $@ is untrusted | ZipSlip.cs:54:72:54:85 | access to property FullName | zip archive | -| ZipSlip.cs:75:57:75:68 | access to local variable destFilePath | Make sure to sanitize relative archive item path before creating path for file extraction if the source of $@ is untrusted | ZipSlip.cs:54:72:54:85 | access to property FullName | zip archive | -| ZipSlip.cs:83:58:83:69 | access to local variable destFilePath | Make sure to sanitize relative archive item path before creating path for file extraction if the source of $@ is untrusted | ZipSlip.cs:54:72:54:85 | access to property FullName | zip archive | +| ZipSlip.cs:24:41:24:52 | access to local variable destFileName | Unsanitized zip archive $@ which may contain '..' used in a file system operation. | ZipSlip.cs:19:31:19:44 | access to property FullName | item path | +| ZipSlip.cs:32:41:32:52 | access to local variable destFilePath | Unsanitized zip archive $@ which may contain '..' used in a file system operation. | ZipSlip.cs:16:52:16:65 | access to property FullName | item path | +| ZipSlip.cs:61:74:61:85 | access to local variable destFilePath | Unsanitized zip archive $@ which may contain '..' used in a file system operation. | ZipSlip.cs:54:72:54:85 | access to property FullName | item path | +| ZipSlip.cs:68:71:68:82 | access to local variable destFilePath | Unsanitized zip archive $@ which may contain '..' used in a file system operation. | ZipSlip.cs:54:72:54:85 | access to property FullName | item path | +| ZipSlip.cs:75:57:75:68 | access to local variable destFilePath | Unsanitized zip archive $@ which may contain '..' used in a file system operation. | ZipSlip.cs:54:72:54:85 | access to property FullName | item path | +| ZipSlip.cs:83:58:83:69 | access to local variable destFilePath | Unsanitized zip archive $@ which may contain '..' used in a file system operation. | ZipSlip.cs:54:72:54:85 | access to property FullName | item path |