From 527fe523a88be1283039ef3cd51091db57b16fb0 Mon Sep 17 00:00:00 2001 From: Tony Torralba Date: Fri, 19 May 2023 16:42:08 +0200 Subject: [PATCH 1/6] Add PathCreation.qll sinks to models-as-data The old PathCreation sinks can't be removed because doing so would cause alert wobble in the path injection queries. See their getReportingNode predicates. --- .../2023-05-19-path-injection-sinks-mad.md | 4 ++++ java/ql/lib/ext/java.io.model.yml | 5 +++++ java/ql/lib/ext/java.nio.file.model.yml | 13 +++++++++---- java/ql/lib/ext/java.nio.model.yml | 1 + .../semmle/code/java/security/TaintedPathQuery.qll | 13 ++----------- java/ql/src/Security/CWE/CWE-022/TaintedPath.ql | 1 + .../ql/src/Security/CWE/CWE-022/TaintedPathLocal.ql | 1 + .../Security/CWE/CWE-073/FilePathInjection.ql | 3 +-- .../security/CWE-073/FilePathInjection.expected | 11 +++++++++++ .../SupportedExternalSinks.expected | 1 + 10 files changed, 36 insertions(+), 17 deletions(-) create mode 100644 java/ql/lib/change-notes/2023-05-19-path-injection-sinks-mad.md diff --git a/java/ql/lib/change-notes/2023-05-19-path-injection-sinks-mad.md b/java/ql/lib/change-notes/2023-05-19-path-injection-sinks-mad.md new file mode 100644 index 00000000000..5f666a0de4f --- /dev/null +++ b/java/ql/lib/change-notes/2023-05-19-path-injection-sinks-mad.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* Path creation sinks modeled in `PathCreation.qll` have been added to the models-as-data sink kinds `create-file` and `read-file`. diff --git a/java/ql/lib/ext/java.io.model.yml b/java/ql/lib/ext/java.io.model.yml index e0920d7df16..83e57a68c74 100644 --- a/java/ql/lib/ext/java.io.model.yml +++ b/java/ql/lib/ext/java.io.model.yml @@ -3,6 +3,10 @@ extensions: pack: codeql/java-all extensible: sinkModel data: + - ["java.io", "File", False, "File", "(File,String)", "", "Argument[1]", "path-injection", "manual"] # old PathCreation + - ["java.io", "File", False, "File", "(String)", "", "Argument[0]", "path-injection", "manual"] # old PathCreation + - ["java.io", "File", False, "File", "(String,String)", "", "Argument[0..1]", "path-injection", "manual"] # old PathCreation + - ["java.io", "File", False, "File", "(URI)", "", "Argument[0]", "path-injection", "manual"] # old PathCreation - ["java.io", "File", True, "createTempFile", "(String,String,File)", "", "Argument[2]", "path-injection", "ai-manual"] - ["java.io", "File", True, "renameTo", "(File)", "", "Argument[0]", "path-injection", "ai-manual"] - ["java.io", "FileInputStream", True, "FileInputStream", "(File)", "", "Argument[0]", "path-injection", "ai-manual"] @@ -11,6 +15,7 @@ extensions: - ["java.io", "FileOutputStream", False, "write", "", "", "Argument[0]", "file-content-store", "manual"] - ["java.io", "FileReader", True, "FileReader", "(File)", "", "Argument[0]", "path-injection", "ai-manual"] - ["java.io", "FileReader", True, "FileReader", "(String)", "", "Argument[0]", "path-injection", "ai-manual"] + - ["java.io", "FileReader", True, "FileReader", "(String,Charset)", "", "Argument[0]", "path-injection", "manual"] - ["java.io", "FileSystem", True, "createDirectory", "(File)", "", "Argument[0]", "path-injection", "ai-manual"] - ["java.io", "FileWriter", False, "FileWriter", "", "", "Argument[0]", "path-injection", "manual"] - ["java.io", "PrintStream", False, "PrintStream", "(File)", "", "Argument[0]", "path-injection", "manual"] diff --git a/java/ql/lib/ext/java.nio.file.model.yml b/java/ql/lib/ext/java.nio.file.model.yml index e4519fbc071..475ddc43eef 100644 --- a/java/ql/lib/ext/java.nio.file.model.yml +++ b/java/ql/lib/ext/java.nio.file.model.yml @@ -3,11 +3,9 @@ extensions: pack: codeql/java-all extensible: sinkModel data: - - ["java.nio.file", "Files", False, "copy", "(Path,OutputStream)", "", "Argument[0]", "path-injection", "manual"] - - ["java.nio.file", "Files", False, "copy", "(Path,Path,CopyOption[])", "", "Argument[0]", "path-injection", "manual"] - - ["java.nio.file", "Files", False, "copy", "(Path,Path,CopyOption[])", "", "Argument[1]", "path-injection", "manual"] + - ["java.nio.file", "Files", False, "copy", "", "", "Argument[0]", "path-injection", "manual"] - ["java.nio.file", "Files", False, "copy", "(InputStream,Path,CopyOption[])", "", "Argument[0]", "file-content-store", "manual"] - - ["java.nio.file", "Files", False, "copy", "(InputStream,Path,CopyOption[])", "", "Argument[1]", "path-injection", "manual"] + - ["java.nio.file", "Files", False, "copy", "", "", "Argument[1]", "path-injection", "manual"] - ["java.nio.file", "Files", False, "createDirectories", "", "", "Argument[0]", "path-injection", "manual"] - ["java.nio.file", "Files", False, "createDirectory", "", "", "Argument[0]", "path-injection", "manual"] - ["java.nio.file", "Files", False, "createFile", "", "", "Argument[0]", "path-injection", "manual"] @@ -40,6 +38,13 @@ extensions: - ["java.nio.file", "Files", True, "delete", "(Path)", "", "Argument[0]", "path-injection", "ai-manual"] - ["java.nio.file", "Files", True, "newInputStream", "(Path,OpenOption[])", "", "Argument[0]", "path-injection", "ai-manual"] - ["java.nio.file", "Files", True, "newOutputStream", "(Path,OpenOption[])", "", "Argument[0]", "path-injection", "ai-manual"] + - ["java.nio.file", "FileSystem", False, "getPath", "", "", "Argument[0..1]", "path-injection", "manual"] # old PathCreation + - ["java.nio.file", "Path", False, "of", "(String,String[])", "", "Argument[0..1]", "path-injection", "manual"] # old PathCreation + - ["java.nio.file", "Path", False, "of", "(URI)", "", "Argument[0]", "path-injection", "manual"] # old PathCreation + - ["java.nio.file", "Path", False, "resolve", "(String)", "", "Argument[0]", "path-injection", "manual"] # old PathCreation + - ["java.nio.file", "Path", False, "resolveSibling", "(String)", "", "Argument[0]", "path-injection", "manual"] # old PathCreation + - ["java.nio.file", "Paths", False, "get", "(String,String[])", "", "Argument[0..1]", "path-injection", "manual"] # old PathCreation + - ["java.nio.file", "Paths", False, "get", "(URI)", "", "Argument[0]", "path-injection", "manual"] # old PathCreation - ["java.nio.file", "SecureDirectoryStream", True, "deleteDirectory", "(Path)", "", "Argument[0]", "path-injection", "ai-manual"] - ["java.nio.file", "SecureDirectoryStream", True, "deleteFile", "(Path)", "", "Argument[0]", "path-injection", "ai-manual"] - addsTo: diff --git a/java/ql/lib/ext/java.nio.model.yml b/java/ql/lib/ext/java.nio.model.yml index 1548dc2c649..9fbe1b253ec 100644 --- a/java/ql/lib/ext/java.nio.model.yml +++ b/java/ql/lib/ext/java.nio.model.yml @@ -6,6 +6,7 @@ extensions: - ["java.nio", "ByteBuffer", False, "array", "()", "", "Argument[this]", "ReturnValue", "taint", "manual"] - ["java.nio", "ByteBuffer", False, "get", "", "", "Argument[this]", "ReturnValue", "taint", "manual"] - ["java.nio", "ByteBuffer", False, "wrap", "(byte[])", "", "Argument[0]", "ReturnValue", "taint", "manual"] + - ["java.nio", "Paths", False, "get", "(URI)", "", "Argument[0]", "ReturnValue", "taint", "manual"] # old PathCreation - addsTo: pack: codeql/java-all diff --git a/java/ql/lib/semmle/code/java/security/TaintedPathQuery.qll b/java/ql/lib/semmle/code/java/security/TaintedPathQuery.qll index 4fa64846c91..a90a23c2165 100644 --- a/java/ql/lib/semmle/code/java/security/TaintedPathQuery.qll +++ b/java/ql/lib/semmle/code/java/security/TaintedPathQuery.qll @@ -5,7 +5,6 @@ import semmle.code.java.frameworks.Networking import semmle.code.java.dataflow.DataFlow import semmle.code.java.dataflow.FlowSources private import semmle.code.java.dataflow.ExternalFlow -import semmle.code.java.security.PathCreation import semmle.code.java.security.PathSanitizer /** @@ -55,11 +54,7 @@ private class TaintPreservingUriCtorParam extends Parameter { module TaintedPathConfig implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource } - predicate isSink(DataFlow::Node sink) { - sink.asExpr() = any(PathCreation p).getAnInput() - or - sinkNode(sink, "path-injection") - } + predicate isSink(DataFlow::Node sink) { sinkNode(sink, "path-injection") } predicate isBarrier(DataFlow::Node sanitizer) { sanitizer.getType() instanceof BoxedType or @@ -82,11 +77,7 @@ module TaintedPathFlow = TaintTracking::Global; module TaintedPathLocalConfig implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node source) { source instanceof LocalUserInput } - predicate isSink(DataFlow::Node sink) { - sink.asExpr() = any(PathCreation p).getAnInput() - or - sinkNode(sink, "path-injection") - } + predicate isSink(DataFlow::Node sink) { sinkNode(sink, "path-injection") } predicate isBarrier(DataFlow::Node sanitizer) { sanitizer.getType() instanceof BoxedType or diff --git a/java/ql/src/Security/CWE/CWE-022/TaintedPath.ql b/java/ql/src/Security/CWE/CWE-022/TaintedPath.ql index 2d73514d97b..96e8e66c7cd 100644 --- a/java/ql/src/Security/CWE/CWE-022/TaintedPath.ql +++ b/java/ql/src/Security/CWE/CWE-022/TaintedPath.ql @@ -14,6 +14,7 @@ */ import java +import semmle.code.java.security.PathCreation import semmle.code.java.security.TaintedPathQuery import TaintedPathFlow::PathGraph diff --git a/java/ql/src/Security/CWE/CWE-022/TaintedPathLocal.ql b/java/ql/src/Security/CWE/CWE-022/TaintedPathLocal.ql index c017b8a3aa9..8e56121883f 100644 --- a/java/ql/src/Security/CWE/CWE-022/TaintedPathLocal.ql +++ b/java/ql/src/Security/CWE/CWE-022/TaintedPathLocal.ql @@ -14,6 +14,7 @@ */ import java +import semmle.code.java.security.PathCreation import semmle.code.java.security.TaintedPathQuery import TaintedPathLocalFlow::PathGraph diff --git a/java/ql/src/experimental/Security/CWE/CWE-073/FilePathInjection.ql b/java/ql/src/experimental/Security/CWE/CWE-073/FilePathInjection.ql index 8e113837bca..ba3411e4da2 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-073/FilePathInjection.ql +++ b/java/ql/src/experimental/Security/CWE/CWE-073/FilePathInjection.ql @@ -16,7 +16,6 @@ import java import semmle.code.java.dataflow.TaintTracking import semmle.code.java.dataflow.ExternalFlow import semmle.code.java.dataflow.FlowSources -import semmle.code.java.security.PathCreation import JFinalController import semmle.code.java.security.PathSanitizer import InjectFilePathFlow::PathGraph @@ -52,7 +51,7 @@ module InjectFilePathConfig implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource } predicate isSink(DataFlow::Node sink) { - sink.asExpr() = any(PathCreation p).getAnInput() and + sinkNode(sink, "path-injection") and not sink instanceof NormalizedPathNode } diff --git a/java/ql/test/experimental/query-tests/security/CWE-073/FilePathInjection.expected b/java/ql/test/experimental/query-tests/security/CWE-073/FilePathInjection.expected index 5720de5c4b9..cd2b49f28c1 100644 --- a/java/ql/test/experimental/query-tests/security/CWE-073/FilePathInjection.expected +++ b/java/ql/test/experimental/query-tests/security/CWE-073/FilePathInjection.expected @@ -2,7 +2,12 @@ edges | FilePathInjection.java:21:21:21:34 | getPara(...) : String | FilePathInjection.java:26:47:26:59 | finalFilePath | | FilePathInjection.java:64:21:64:34 | getPara(...) : String | FilePathInjection.java:72:47:72:59 | finalFilePath | | FilePathInjection.java:87:21:87:34 | getPara(...) : String | FilePathInjection.java:95:47:95:59 | finalFilePath | +| FilePathInjection.java:177:50:177:58 | file : File | FilePathInjection.java:182:30:182:33 | file | | FilePathInjection.java:205:17:205:44 | getParameter(...) : String | FilePathInjection.java:209:24:209:31 | filePath | +| FilePathInjection.java:205:17:205:44 | getParameter(...) : String | FilePathInjection.java:209:24:209:31 | filePath : String | +| FilePathInjection.java:209:15:209:32 | new File(...) : File | FilePathInjection.java:217:19:217:22 | file : File | +| FilePathInjection.java:209:24:209:31 | filePath : String | FilePathInjection.java:209:15:209:32 | new File(...) : File | +| FilePathInjection.java:217:19:217:22 | file : File | FilePathInjection.java:177:50:177:58 | file : File | nodes | FilePathInjection.java:21:21:21:34 | getPara(...) : String | semmle.label | getPara(...) : String | | FilePathInjection.java:26:47:26:59 | finalFilePath | semmle.label | finalFilePath | @@ -10,11 +15,17 @@ nodes | FilePathInjection.java:72:47:72:59 | finalFilePath | semmle.label | finalFilePath | | FilePathInjection.java:87:21:87:34 | getPara(...) : String | semmle.label | getPara(...) : String | | FilePathInjection.java:95:47:95:59 | finalFilePath | semmle.label | finalFilePath | +| FilePathInjection.java:177:50:177:58 | file : File | semmle.label | file : File | +| FilePathInjection.java:182:30:182:33 | file | semmle.label | file | | FilePathInjection.java:205:17:205:44 | getParameter(...) : String | semmle.label | getParameter(...) : String | +| FilePathInjection.java:209:15:209:32 | new File(...) : File | semmle.label | new File(...) : File | | FilePathInjection.java:209:24:209:31 | filePath | semmle.label | filePath | +| FilePathInjection.java:209:24:209:31 | filePath : String | semmle.label | filePath : String | +| FilePathInjection.java:217:19:217:22 | file : File | semmle.label | file : File | subpaths #select | FilePathInjection.java:26:47:26:59 | finalFilePath | FilePathInjection.java:21:21:21:34 | getPara(...) : String | FilePathInjection.java:26:47:26:59 | finalFilePath | External control of file name or path due to $@. | FilePathInjection.java:21:21:21:34 | getPara(...) | user-provided value | | FilePathInjection.java:72:47:72:59 | finalFilePath | FilePathInjection.java:64:21:64:34 | getPara(...) : String | FilePathInjection.java:72:47:72:59 | finalFilePath | External control of file name or path due to $@. | FilePathInjection.java:64:21:64:34 | getPara(...) | user-provided value | | FilePathInjection.java:95:47:95:59 | finalFilePath | FilePathInjection.java:87:21:87:34 | getPara(...) : String | FilePathInjection.java:95:47:95:59 | finalFilePath | External control of file name or path due to $@. | FilePathInjection.java:87:21:87:34 | getPara(...) | user-provided value | +| FilePathInjection.java:182:30:182:33 | file | FilePathInjection.java:205:17:205:44 | getParameter(...) : String | FilePathInjection.java:182:30:182:33 | file | External control of file name or path due to $@. | FilePathInjection.java:205:17:205:44 | getParameter(...) | user-provided value | | FilePathInjection.java:209:24:209:31 | filePath | FilePathInjection.java:205:17:205:44 | getParameter(...) : String | FilePathInjection.java:209:24:209:31 | filePath | External control of file name or path due to $@. | FilePathInjection.java:205:17:205:44 | getParameter(...) | user-provided value | diff --git a/java/ql/test/query-tests/Telemetry/SupportedExternalSinks/SupportedExternalSinks.expected b/java/ql/test/query-tests/Telemetry/SupportedExternalSinks/SupportedExternalSinks.expected index 6cb849601d5..5f0ed7d05df 100644 --- a/java/ql/test/query-tests/Telemetry/SupportedExternalSinks/SupportedExternalSinks.expected +++ b/java/ql/test/query-tests/Telemetry/SupportedExternalSinks/SupportedExternalSinks.expected @@ -1,2 +1,3 @@ +| java.io.File#File(String) | 1 | | java.io.FileWriter#FileWriter(File) | 1 | | java.net.URL#openStream() | 1 | From 1ccec90c6f5d2379f51797d29e2235466894f669 Mon Sep 17 00:00:00 2001 From: Tony Torralba Date: Tue, 6 Jun 2023 09:10:18 +0200 Subject: [PATCH 2/6] Apply suggestions from code review Co-authored-by: Jami <57204504+jcogs33@users.noreply.github.com> --- java/ql/lib/change-notes/2023-05-19-path-injection-sinks-mad.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/java/ql/lib/change-notes/2023-05-19-path-injection-sinks-mad.md b/java/ql/lib/change-notes/2023-05-19-path-injection-sinks-mad.md index 5f666a0de4f..ae5cd306c2b 100644 --- a/java/ql/lib/change-notes/2023-05-19-path-injection-sinks-mad.md +++ b/java/ql/lib/change-notes/2023-05-19-path-injection-sinks-mad.md @@ -1,4 +1,4 @@ --- category: minorAnalysis --- -* Path creation sinks modeled in `PathCreation.qll` have been added to the models-as-data sink kinds `create-file` and `read-file`. +* Path creation sinks modeled in `PathCreation.qll` have been added to the models-as-data sink kind `path-injection`. From 0065e6e1d6efeb95b7a94a79cad40ad4a2a761cf Mon Sep 17 00:00:00 2001 From: Tony Torralba Date: Tue, 6 Jun 2023 10:04:22 +0200 Subject: [PATCH 3/6] Apply suggestions from code review Fix incorrect models-as-data rows --- java/ql/lib/ext/java.nio.file.model.yml | 6 ++++-- java/ql/lib/ext/java.nio.model.yml | 1 - 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/java/ql/lib/ext/java.nio.file.model.yml b/java/ql/lib/ext/java.nio.file.model.yml index 475ddc43eef..f27f6a249a1 100644 --- a/java/ql/lib/ext/java.nio.file.model.yml +++ b/java/ql/lib/ext/java.nio.file.model.yml @@ -3,9 +3,11 @@ extensions: pack: codeql/java-all extensible: sinkModel data: - - ["java.nio.file", "Files", False, "copy", "", "", "Argument[0]", "path-injection", "manual"] + - ["java.nio.file", "Files", False, "copy", "(Path,OutputStream)", "", "Argument[0]", "path-injection", "manual"] + - ["java.nio.file", "Files", False, "copy", "(Path,Path,CopyOption[])", "", "Argument[0]", "path-injection", "manual"] + - ["java.nio.file", "Files", False, "copy", "(Path,Path,CopyOption[])", "", "Argument[1]", "path-injection", "manual"] - ["java.nio.file", "Files", False, "copy", "(InputStream,Path,CopyOption[])", "", "Argument[0]", "file-content-store", "manual"] - - ["java.nio.file", "Files", False, "copy", "", "", "Argument[1]", "path-injection", "manual"] + - ["java.nio.file", "Files", False, "copy", "(InputStream,Path,CopyOption[])", "", "Argument[1]", "path-injection", "manual"] - ["java.nio.file", "Files", False, "createDirectories", "", "", "Argument[0]", "path-injection", "manual"] - ["java.nio.file", "Files", False, "createDirectory", "", "", "Argument[0]", "path-injection", "manual"] - ["java.nio.file", "Files", False, "createFile", "", "", "Argument[0]", "path-injection", "manual"] diff --git a/java/ql/lib/ext/java.nio.model.yml b/java/ql/lib/ext/java.nio.model.yml index 9fbe1b253ec..1548dc2c649 100644 --- a/java/ql/lib/ext/java.nio.model.yml +++ b/java/ql/lib/ext/java.nio.model.yml @@ -6,7 +6,6 @@ extensions: - ["java.nio", "ByteBuffer", False, "array", "()", "", "Argument[this]", "ReturnValue", "taint", "manual"] - ["java.nio", "ByteBuffer", False, "get", "", "", "Argument[this]", "ReturnValue", "taint", "manual"] - ["java.nio", "ByteBuffer", False, "wrap", "(byte[])", "", "Argument[0]", "ReturnValue", "taint", "manual"] - - ["java.nio", "Paths", False, "get", "(URI)", "", "Argument[0]", "ReturnValue", "taint", "manual"] # old PathCreation - addsTo: pack: codeql/java-all From 1601846478e04f082886014baf0d7cadfc17a991 Mon Sep 17 00:00:00 2001 From: Tony Torralba Date: Tue, 6 Jun 2023 10:06:06 +0200 Subject: [PATCH 4/6] Add exclusion to the ZipSlip query to avoid FPs --- .../code/java/security/ZipSlipQuery.qll | 23 ++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/java/ql/lib/semmle/code/java/security/ZipSlipQuery.qll b/java/ql/lib/semmle/code/java/security/ZipSlipQuery.qll index 4fad191a3e4..68365db51c2 100644 --- a/java/ql/lib/semmle/code/java/security/ZipSlipQuery.qll +++ b/java/ql/lib/semmle/code/java/security/ZipSlipQuery.qll @@ -4,6 +4,7 @@ import java import semmle.code.java.dataflow.TaintTracking import semmle.code.java.security.PathSanitizer private import semmle.code.java.dataflow.ExternalFlow +private import semmle.code.java.security.PathCreation /** * A method that returns the name of an archive entry. @@ -40,5 +41,25 @@ module ZipSlipFlow = TaintTracking::Global; * A sink that represents a file creation, such as a file write, copy or move operation. */ private class FileCreationSink extends DataFlow::Node { - FileCreationSink() { sinkNode(this, "path-injection") } + FileCreationSink() { + sinkNode(this, "path-injection") and + not isPathCreation(this) + } +} + +/** + * Holds if `sink` is a path creation node that doesn't imply a read/write filesystem operation. + * This is to avoid creating new spurious alerts, since `PathCreation` sinks weren't + * previosuly part of this query. + */ +private predicate isPathCreation(DataFlow::Node sink) { + exists(PathCreation pc | + pc.getAnInput() = sink.asExpr() and + // exclude actual read/write operations included in `PathCreation` + not pc.(Call) + .getCallee() + .getDeclaringType() + .hasQualifiedName("java.io", + ["FileInputStream", "FileOutputStream", "FileReader", "FileWriter"]) + ) } From 8001ae9669ffe778732f2b5b4b2ff83d81f23b14 Mon Sep 17 00:00:00 2001 From: Tony Torralba Date: Wed, 7 Jun 2023 09:08:24 +0200 Subject: [PATCH 5/6] Update java/ql/lib/semmle/code/java/security/ZipSlipQuery.qll Co-authored-by: Jami <57204504+jcogs33@users.noreply.github.com> --- java/ql/lib/semmle/code/java/security/ZipSlipQuery.qll | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/java/ql/lib/semmle/code/java/security/ZipSlipQuery.qll b/java/ql/lib/semmle/code/java/security/ZipSlipQuery.qll index 68365db51c2..8261aef9fb3 100644 --- a/java/ql/lib/semmle/code/java/security/ZipSlipQuery.qll +++ b/java/ql/lib/semmle/code/java/security/ZipSlipQuery.qll @@ -50,7 +50,7 @@ private class FileCreationSink extends DataFlow::Node { /** * Holds if `sink` is a path creation node that doesn't imply a read/write filesystem operation. * This is to avoid creating new spurious alerts, since `PathCreation` sinks weren't - * previosuly part of this query. + * previously part of this query. */ private predicate isPathCreation(DataFlow::Node sink) { exists(PathCreation pc | From 27763d6bbed3823c91058f1bd82834b1fc3c1da6 Mon Sep 17 00:00:00 2001 From: Tony Torralba Date: Wed, 7 Jun 2023 09:25:42 +0200 Subject: [PATCH 6/6] Improve ZipSlip exclusion to take varargs into account --- java/ql/lib/semmle/code/java/security/ZipSlipQuery.qll | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/java/ql/lib/semmle/code/java/security/ZipSlipQuery.qll b/java/ql/lib/semmle/code/java/security/ZipSlipQuery.qll index 8261aef9fb3..074153ffd8f 100644 --- a/java/ql/lib/semmle/code/java/security/ZipSlipQuery.qll +++ b/java/ql/lib/semmle/code/java/security/ZipSlipQuery.qll @@ -54,7 +54,10 @@ private class FileCreationSink extends DataFlow::Node { */ private predicate isPathCreation(DataFlow::Node sink) { exists(PathCreation pc | - pc.getAnInput() = sink.asExpr() and + pc.getAnInput() = sink.asExpr() + or + pc.getAnInput().(Argument).isVararg() and sink.(DataFlow::ImplicitVarargsArray).getCall() = pc + | // exclude actual read/write operations included in `PathCreation` not pc.(Call) .getCallee()