From 7a1aead83185c97759ef80dc1ba3ab065975e8a3 Mon Sep 17 00:00:00 2001 From: Asger F Date: Wed, 4 Oct 2023 22:12:06 +0200 Subject: [PATCH] JS: Port ZipSlip --- .../security/dataflow/ZipSlipQuery.qll | 36 ++++- javascript/ql/src/Security/CWE-022/ZipSlip.ql | 6 +- .../Security/CWE-022/ZipSlip/ZipSlip.expected | 136 ++++-------------- 3 files changed, 61 insertions(+), 117 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/ZipSlipQuery.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/ZipSlipQuery.qll index 9aad934759d..87da9d2b325 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/ZipSlipQuery.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/ZipSlipQuery.qll @@ -20,7 +20,39 @@ private class ConcreteSplitPath extends TaintedPath::Label::SplitPath { } /** A taint tracking configuration for unsafe archive extraction. */ -class Configuration extends DataFlow::Configuration { +module ZipSlipConfig implements DataFlow::StateConfigSig { + class FlowState = DataFlow::FlowLabel; + + predicate isSource(DataFlow::Node source, DataFlow::FlowLabel label) { + label = source.(Source).getAFlowLabel() + } + + predicate isSink(DataFlow::Node sink, DataFlow::FlowLabel label) { + label = sink.(Sink).getAFlowLabel() + } + + predicate isBarrier(DataFlow::Node node) { + node instanceof TaintedPath::Sanitizer or + node = DataFlow::MakeBarrierGuard::getABarrierNode() + } + + predicate isBarrier(DataFlow::Node node, DataFlow::FlowLabel label) { + node = DataFlow::MakeLabeledBarrierGuard::getABarrierNode(label) + } + + predicate isAdditionalFlowStep( + DataFlow::Node node1, DataFlow::FlowLabel state1, DataFlow::Node node2, + DataFlow::FlowLabel state2 + ) { + TaintedPath::isAdditionalTaintedPathFlowStep(node1, node2, state1, state2) + } +} + +/** A taint tracking configuration for unsafe archive extraction. */ +module ZipSlipFlow = DataFlow::GlobalWithState; + +/** A taint tracking configuration for unsafe archive extraction. */ +deprecated class Configuration extends DataFlow::Configuration { Configuration() { this = "ZipSlip" } override predicate isSource(DataFlow::Node source, DataFlow::FlowLabel label) { @@ -44,6 +76,6 @@ class Configuration extends DataFlow::Configuration { DataFlow::Node src, DataFlow::Node dst, DataFlow::FlowLabel srclabel, DataFlow::FlowLabel dstlabel ) { - TaintedPath::isAdditionalTaintedPathFlowStep(src, dst, srclabel, dstlabel) + ZipSlipConfig::isAdditionalFlowStep(src, srclabel, dst, dstlabel) } } diff --git a/javascript/ql/src/Security/CWE-022/ZipSlip.ql b/javascript/ql/src/Security/CWE-022/ZipSlip.ql index aef13830eb1..e2f13d0e1f6 100644 --- a/javascript/ql/src/Security/CWE-022/ZipSlip.ql +++ b/javascript/ql/src/Security/CWE-022/ZipSlip.ql @@ -14,10 +14,10 @@ import javascript import semmle.javascript.security.dataflow.ZipSlipQuery -import DataFlow::PathGraph +import DataFlow::DeduplicatePathGraph -from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink -where cfg.hasFlowPath(source, sink) +from PathNode source, PathNode sink +where ZipSlipFlow::flowPath(source.getAnOriginalPathNode(), sink.getAnOriginalPathNode()) select source.getNode(), source, sink, "Unsanitized archive entry, which may contain '..', is used in a $@.", sink.getNode(), "file system operation" diff --git a/javascript/ql/test/query-tests/Security/CWE-022/ZipSlip/ZipSlip.expected b/javascript/ql/test/query-tests/Security/CWE-022/ZipSlip/ZipSlip.expected index 253bca10b03..9b147acdd88 100644 --- a/javascript/ql/test/query-tests/Security/CWE-022/ZipSlip/ZipSlip.expected +++ b/javascript/ql/test/query-tests/Security/CWE-022/ZipSlip/ZipSlip.expected @@ -1,130 +1,42 @@ nodes -| AdmZipBad.js:6:24:6:41 | zipEntry.entryName | -| AdmZipBad.js:6:24:6:41 | zipEntry.entryName | -| AdmZipBad.js:6:24:6:41 | zipEntry.entryName | -| AdmZipBad.js:6:24:6:41 | zipEntry.entryName | -| TarSlipBad.js:6:36:6:46 | header.name | -| TarSlipBad.js:6:36:6:46 | header.name | -| TarSlipBad.js:6:36:6:46 | header.name | -| TarSlipBad.js:6:36:6:46 | header.name | -| TarSlipBad.js:9:17:9:31 | header.linkname | -| TarSlipBad.js:9:17:9:31 | header.linkname | -| TarSlipBad.js:9:17:9:31 | header.linkname | -| TarSlipBad.js:9:17:9:31 | header.linkname | -| ZipSlipBad2.js:5:9:5:46 | fileName | -| ZipSlipBad2.js:5:9:5:46 | fileName | -| ZipSlipBad2.js:5:20:5:46 | 'output ... ry.path | -| ZipSlipBad2.js:5:20:5:46 | 'output ... ry.path | -| ZipSlipBad2.js:5:37:5:46 | entry.path | -| ZipSlipBad2.js:5:37:5:46 | entry.path | -| ZipSlipBad2.js:5:37:5:46 | entry.path | -| ZipSlipBad2.js:6:22:6:29 | fileName | -| ZipSlipBad2.js:6:22:6:29 | fileName | -| ZipSlipBad2.js:6:22:6:29 | fileName | -| ZipSlipBad.js:7:11:7:31 | fileName | -| ZipSlipBad.js:7:11:7:31 | fileName | -| ZipSlipBad.js:7:22:7:31 | entry.path | -| ZipSlipBad.js:7:22:7:31 | entry.path | -| ZipSlipBad.js:7:22:7:31 | entry.path | -| ZipSlipBad.js:8:37:8:44 | fileName | -| ZipSlipBad.js:8:37:8:44 | fileName | -| ZipSlipBad.js:8:37:8:44 | fileName | -| ZipSlipBad.js:15:11:15:31 | fileName | -| ZipSlipBad.js:15:11:15:31 | fileName | -| ZipSlipBad.js:15:22:15:31 | entry.path | -| ZipSlipBad.js:15:22:15:31 | entry.path | -| ZipSlipBad.js:15:22:15:31 | entry.path | -| ZipSlipBad.js:16:30:16:37 | fileName | -| ZipSlipBad.js:16:30:16:37 | fileName | -| ZipSlipBad.js:16:30:16:37 | fileName | -| ZipSlipBad.js:22:11:22:31 | fileName | -| ZipSlipBad.js:22:11:22:31 | fileName | -| ZipSlipBad.js:22:22:22:31 | entry.path | -| ZipSlipBad.js:22:22:22:31 | entry.path | -| ZipSlipBad.js:22:22:22:31 | entry.path | -| ZipSlipBad.js:23:28:23:35 | fileName | -| ZipSlipBad.js:23:28:23:35 | fileName | -| ZipSlipBad.js:23:28:23:35 | fileName | -| ZipSlipBad.js:30:14:30:17 | name | -| ZipSlipBad.js:30:14:30:17 | name | -| ZipSlipBad.js:30:14:30:17 | name | -| ZipSlipBad.js:31:26:31:29 | name | -| ZipSlipBad.js:31:26:31:29 | name | -| ZipSlipBad.js:31:26:31:29 | name | -| ZipSlipBad.js:34:16:34:19 | name | -| ZipSlipBad.js:34:16:34:19 | name | -| ZipSlipBad.js:34:16:34:19 | name | -| ZipSlipBad.js:35:26:35:29 | name | -| ZipSlipBad.js:35:26:35:29 | name | -| ZipSlipBad.js:35:26:35:29 | name | -| ZipSlipBadUnzipper.js:7:9:7:29 | fileName | -| ZipSlipBadUnzipper.js:7:9:7:29 | fileName | -| ZipSlipBadUnzipper.js:7:20:7:29 | entry.path | -| ZipSlipBadUnzipper.js:7:20:7:29 | entry.path | -| ZipSlipBadUnzipper.js:7:20:7:29 | entry.path | -| ZipSlipBadUnzipper.js:8:37:8:44 | fileName | -| ZipSlipBadUnzipper.js:8:37:8:44 | fileName | -| ZipSlipBadUnzipper.js:8:37:8:44 | fileName | +| AdmZipBad.js:6:24:6:41 | zipEntry.entryName | semmle.label | zipEntry.entryName | +| TarSlipBad.js:6:36:6:46 | header.name | semmle.label | header.name | +| TarSlipBad.js:9:17:9:31 | header.linkname | semmle.label | header.linkname | +| ZipSlipBad2.js:5:9:5:46 | fileName | semmle.label | fileName | +| ZipSlipBad2.js:5:20:5:46 | 'output ... ry.path | semmle.label | 'output ... ry.path | +| ZipSlipBad2.js:5:37:5:46 | entry.path | semmle.label | entry.path | +| ZipSlipBad2.js:6:22:6:29 | fileName | semmle.label | fileName | +| ZipSlipBad.js:7:11:7:31 | fileName | semmle.label | fileName | +| ZipSlipBad.js:7:22:7:31 | entry.path | semmle.label | entry.path | +| ZipSlipBad.js:8:37:8:44 | fileName | semmle.label | fileName | +| ZipSlipBad.js:15:11:15:31 | fileName | semmle.label | fileName | +| ZipSlipBad.js:15:22:15:31 | entry.path | semmle.label | entry.path | +| ZipSlipBad.js:16:30:16:37 | fileName | semmle.label | fileName | +| ZipSlipBad.js:22:11:22:31 | fileName | semmle.label | fileName | +| ZipSlipBad.js:22:22:22:31 | entry.path | semmle.label | entry.path | +| ZipSlipBad.js:23:28:23:35 | fileName | semmle.label | fileName | +| ZipSlipBad.js:30:14:30:17 | name | semmle.label | name | +| ZipSlipBad.js:31:26:31:29 | name | semmle.label | name | +| ZipSlipBad.js:34:16:34:19 | name | semmle.label | name | +| ZipSlipBad.js:35:26:35:29 | name | semmle.label | name | +| ZipSlipBadUnzipper.js:7:9:7:29 | fileName | semmle.label | fileName | +| ZipSlipBadUnzipper.js:7:20:7:29 | entry.path | semmle.label | entry.path | +| ZipSlipBadUnzipper.js:8:37:8:44 | fileName | semmle.label | fileName | edges -| AdmZipBad.js:6:24:6:41 | zipEntry.entryName | AdmZipBad.js:6:24:6:41 | zipEntry.entryName | -| TarSlipBad.js:6:36:6:46 | header.name | TarSlipBad.js:6:36:6:46 | header.name | -| TarSlipBad.js:9:17:9:31 | header.linkname | TarSlipBad.js:9:17:9:31 | header.linkname | -| ZipSlipBad2.js:5:9:5:46 | fileName | ZipSlipBad2.js:6:22:6:29 | fileName | -| ZipSlipBad2.js:5:9:5:46 | fileName | ZipSlipBad2.js:6:22:6:29 | fileName | -| ZipSlipBad2.js:5:9:5:46 | fileName | ZipSlipBad2.js:6:22:6:29 | fileName | | ZipSlipBad2.js:5:9:5:46 | fileName | ZipSlipBad2.js:6:22:6:29 | fileName | | ZipSlipBad2.js:5:20:5:46 | 'output ... ry.path | ZipSlipBad2.js:5:9:5:46 | fileName | -| ZipSlipBad2.js:5:20:5:46 | 'output ... ry.path | ZipSlipBad2.js:5:9:5:46 | fileName | -| ZipSlipBad2.js:5:37:5:46 | entry.path | ZipSlipBad2.js:5:20:5:46 | 'output ... ry.path | -| ZipSlipBad2.js:5:37:5:46 | entry.path | ZipSlipBad2.js:5:20:5:46 | 'output ... ry.path | -| ZipSlipBad2.js:5:37:5:46 | entry.path | ZipSlipBad2.js:5:20:5:46 | 'output ... ry.path | | ZipSlipBad2.js:5:37:5:46 | entry.path | ZipSlipBad2.js:5:20:5:46 | 'output ... ry.path | | ZipSlipBad.js:7:11:7:31 | fileName | ZipSlipBad.js:8:37:8:44 | fileName | -| ZipSlipBad.js:7:11:7:31 | fileName | ZipSlipBad.js:8:37:8:44 | fileName | -| ZipSlipBad.js:7:11:7:31 | fileName | ZipSlipBad.js:8:37:8:44 | fileName | -| ZipSlipBad.js:7:11:7:31 | fileName | ZipSlipBad.js:8:37:8:44 | fileName | -| ZipSlipBad.js:7:22:7:31 | entry.path | ZipSlipBad.js:7:11:7:31 | fileName | -| ZipSlipBad.js:7:22:7:31 | entry.path | ZipSlipBad.js:7:11:7:31 | fileName | -| ZipSlipBad.js:7:22:7:31 | entry.path | ZipSlipBad.js:7:11:7:31 | fileName | | ZipSlipBad.js:7:22:7:31 | entry.path | ZipSlipBad.js:7:11:7:31 | fileName | | ZipSlipBad.js:15:11:15:31 | fileName | ZipSlipBad.js:16:30:16:37 | fileName | -| ZipSlipBad.js:15:11:15:31 | fileName | ZipSlipBad.js:16:30:16:37 | fileName | -| ZipSlipBad.js:15:11:15:31 | fileName | ZipSlipBad.js:16:30:16:37 | fileName | -| ZipSlipBad.js:15:11:15:31 | fileName | ZipSlipBad.js:16:30:16:37 | fileName | -| ZipSlipBad.js:15:22:15:31 | entry.path | ZipSlipBad.js:15:11:15:31 | fileName | -| ZipSlipBad.js:15:22:15:31 | entry.path | ZipSlipBad.js:15:11:15:31 | fileName | -| ZipSlipBad.js:15:22:15:31 | entry.path | ZipSlipBad.js:15:11:15:31 | fileName | | ZipSlipBad.js:15:22:15:31 | entry.path | ZipSlipBad.js:15:11:15:31 | fileName | | ZipSlipBad.js:22:11:22:31 | fileName | ZipSlipBad.js:23:28:23:35 | fileName | -| ZipSlipBad.js:22:11:22:31 | fileName | ZipSlipBad.js:23:28:23:35 | fileName | -| ZipSlipBad.js:22:11:22:31 | fileName | ZipSlipBad.js:23:28:23:35 | fileName | -| ZipSlipBad.js:22:11:22:31 | fileName | ZipSlipBad.js:23:28:23:35 | fileName | -| ZipSlipBad.js:22:22:22:31 | entry.path | ZipSlipBad.js:22:11:22:31 | fileName | -| ZipSlipBad.js:22:22:22:31 | entry.path | ZipSlipBad.js:22:11:22:31 | fileName | -| ZipSlipBad.js:22:22:22:31 | entry.path | ZipSlipBad.js:22:11:22:31 | fileName | | ZipSlipBad.js:22:22:22:31 | entry.path | ZipSlipBad.js:22:11:22:31 | fileName | | ZipSlipBad.js:30:14:30:17 | name | ZipSlipBad.js:31:26:31:29 | name | -| ZipSlipBad.js:30:14:30:17 | name | ZipSlipBad.js:31:26:31:29 | name | -| ZipSlipBad.js:30:14:30:17 | name | ZipSlipBad.js:31:26:31:29 | name | -| ZipSlipBad.js:30:14:30:17 | name | ZipSlipBad.js:31:26:31:29 | name | -| ZipSlipBad.js:30:14:30:17 | name | ZipSlipBad.js:31:26:31:29 | name | -| ZipSlipBad.js:30:14:30:17 | name | ZipSlipBad.js:31:26:31:29 | name | -| ZipSlipBad.js:30:14:30:17 | name | ZipSlipBad.js:31:26:31:29 | name | -| ZipSlipBad.js:34:16:34:19 | name | ZipSlipBad.js:35:26:35:29 | name | -| ZipSlipBad.js:34:16:34:19 | name | ZipSlipBad.js:35:26:35:29 | name | -| ZipSlipBad.js:34:16:34:19 | name | ZipSlipBad.js:35:26:35:29 | name | -| ZipSlipBad.js:34:16:34:19 | name | ZipSlipBad.js:35:26:35:29 | name | -| ZipSlipBad.js:34:16:34:19 | name | ZipSlipBad.js:35:26:35:29 | name | -| ZipSlipBad.js:34:16:34:19 | name | ZipSlipBad.js:35:26:35:29 | name | | ZipSlipBad.js:34:16:34:19 | name | ZipSlipBad.js:35:26:35:29 | name | | ZipSlipBadUnzipper.js:7:9:7:29 | fileName | ZipSlipBadUnzipper.js:8:37:8:44 | fileName | -| ZipSlipBadUnzipper.js:7:9:7:29 | fileName | ZipSlipBadUnzipper.js:8:37:8:44 | fileName | -| ZipSlipBadUnzipper.js:7:9:7:29 | fileName | ZipSlipBadUnzipper.js:8:37:8:44 | fileName | -| ZipSlipBadUnzipper.js:7:9:7:29 | fileName | ZipSlipBadUnzipper.js:8:37:8:44 | fileName | -| ZipSlipBadUnzipper.js:7:20:7:29 | entry.path | ZipSlipBadUnzipper.js:7:9:7:29 | fileName | -| ZipSlipBadUnzipper.js:7:20:7:29 | entry.path | ZipSlipBadUnzipper.js:7:9:7:29 | fileName | -| ZipSlipBadUnzipper.js:7:20:7:29 | entry.path | ZipSlipBadUnzipper.js:7:9:7:29 | fileName | | ZipSlipBadUnzipper.js:7:20:7:29 | entry.path | ZipSlipBadUnzipper.js:7:9:7:29 | fileName | +subpaths #select | AdmZipBad.js:6:24:6:41 | zipEntry.entryName | AdmZipBad.js:6:24:6:41 | zipEntry.entryName | AdmZipBad.js:6:24:6:41 | zipEntry.entryName | Unsanitized archive entry, which may contain '..', is used in a $@. | AdmZipBad.js:6:24:6:41 | zipEntry.entryName | file system operation | | TarSlipBad.js:6:36:6:46 | header.name | TarSlipBad.js:6:36:6:46 | header.name | TarSlipBad.js:6:36:6:46 | header.name | Unsanitized archive entry, which may contain '..', is used in a $@. | TarSlipBad.js:6:36:6:46 | header.name | file system operation |