diff --git a/python/ql/src/experimental/Security/CWE-022bis/TarSlipImprov.ql b/python/ql/src/experimental/Security/CWE-022bis/TarSlipImprov.ql index 65ff272bbbd..431fe293cec 100755 --- a/python/ql/src/experimental/Security/CWE-022bis/TarSlipImprov.ql +++ b/python/ql/src/experimental/Security/CWE-022bis/TarSlipImprov.ql @@ -16,7 +16,7 @@ import python import semmle.python.dataflow.new.DataFlow import semmle.python.dataflow.new.TaintTracking -import DataFlow::PathGraph +import TarSlipImprovFlow::PathGraph import semmle.python.ApiGraphs import semmle.python.dataflow.new.internal.Attributes import semmle.python.dataflow.new.BarrierGuards @@ -54,12 +54,10 @@ class AllTarfileOpens extends API::CallNode { /** * A taint-tracking configuration for detecting more "TarSlip" vulnerabilities. */ -class Configuration extends TaintTracking::Configuration { - Configuration() { this = "TarSlip" } +private module TarSlipImprovConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { source = tarfileOpen().getACall() } - override predicate isSource(DataFlow::Node source) { source = tarfileOpen().getACall() } - - override predicate isSink(DataFlow::Node sink) { + predicate isSink(DataFlow::Node sink) { ( // A sink capturing method calls to `extractall` without `members` argument. // For a call to `file.extractall` without `members` argument, `file` is considered a sink. @@ -100,7 +98,7 @@ class Configuration extends TaintTracking::Configuration { not sink.getScope().getLocation().getFile().inStdlib() } - override predicate isAdditionalTaintStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) { + predicate isAdditionalFlowStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) { nodeTo.(MethodCallNode).calls(nodeFrom, "getmembers") and nodeFrom instanceof AllTarfileOpens or @@ -113,7 +111,10 @@ class Configuration extends TaintTracking::Configuration { } } -from Configuration config, DataFlow::PathNode source, DataFlow::PathNode sink -where config.hasFlowPath(source, sink) +/** Global taint-tracking for detecting more "TarSlip" vulnerabilities. */ +module TarSlipImprovFlow = TaintTracking::Global; + +from TarSlipImprovFlow::PathNode source, TarSlipImprovFlow::PathNode sink +where TarSlipImprovFlow::flowPath(source, sink) select sink, source, sink, "Extraction of tarfile from $@ to a potentially untrusted source $@.", source.getNode(), source.getNode().toString(), sink.getNode(), sink.getNode().toString() diff --git a/python/ql/test/experimental/query-tests/Security/CWE-022-TarSlip/TarSlip.expected b/python/ql/test/experimental/query-tests/Security/CWE-022-TarSlip/TarSlip.expected index 52fd49e57b1..a699a1bac5f 100644 --- a/python/ql/test/experimental/query-tests/Security/CWE-022-TarSlip/TarSlip.expected +++ b/python/ql/test/experimental/query-tests/Security/CWE-022-TarSlip/TarSlip.expected @@ -33,7 +33,8 @@ edges | TarSlipImprov.py:141:34:141:36 | GSSA Variable tar | TarSlipImprov.py:142:9:142:13 | GSSA Variable entry | | TarSlipImprov.py:142:9:142:13 | GSSA Variable entry | TarSlipImprov.py:143:36:143:40 | ControlFlowNode for entry | | TarSlipImprov.py:159:9:159:14 | SSA variable tar_cm | TarSlipImprov.py:162:20:162:23 | SSA variable tarc | -| TarSlipImprov.py:159:26:159:51 | ControlFlowNode for Attribute() | TarSlipImprov.py:159:9:159:14 | SSA variable tar_cm | +| TarSlipImprov.py:159:18:159:52 | ControlFlowNode for closing() | TarSlipImprov.py:159:9:159:14 | SSA variable tar_cm | +| TarSlipImprov.py:159:26:159:51 | ControlFlowNode for Attribute() | TarSlipImprov.py:159:18:159:52 | ControlFlowNode for closing() | | TarSlipImprov.py:162:20:162:23 | SSA variable tarc | TarSlipImprov.py:169:9:169:12 | ControlFlowNode for tarc | | TarSlipImprov.py:176:6:176:31 | ControlFlowNode for Attribute() | TarSlipImprov.py:176:36:176:38 | GSSA Variable tar | | TarSlipImprov.py:176:36:176:38 | GSSA Variable tar | TarSlipImprov.py:177:9:177:13 | GSSA Variable entry | @@ -122,6 +123,7 @@ nodes | TarSlipImprov.py:142:9:142:13 | GSSA Variable entry | semmle.label | GSSA Variable entry | | TarSlipImprov.py:143:36:143:40 | ControlFlowNode for entry | semmle.label | ControlFlowNode for entry | | TarSlipImprov.py:159:9:159:14 | SSA variable tar_cm | semmle.label | SSA variable tar_cm | +| TarSlipImprov.py:159:18:159:52 | ControlFlowNode for closing() | semmle.label | ControlFlowNode for closing() | | TarSlipImprov.py:159:26:159:51 | ControlFlowNode for Attribute() | semmle.label | ControlFlowNode for Attribute() | | TarSlipImprov.py:162:20:162:23 | SSA variable tarc | semmle.label | SSA variable tarc | | TarSlipImprov.py:169:9:169:12 | ControlFlowNode for tarc | semmle.label | ControlFlowNode for tarc |