Merge pull request #11854 from yoff/python/fix-tarslip-improv-bug

Python: fix bug  in `py/tarslip-extended`

Author: yoff

python/ql/src/experimental/Security/CWE-022bis/TarSlipImprov.ql

@@ -101,19 +101,15 @@ class Configuration extends TaintTracking::Configuration {
   }
 
   override predicate isAdditionalTaintStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
-    exists(AttrRead attr, MethodCallNode call |
-      attr.accesses(nodeFrom, "getmembers") and
-      nodeFrom = call.getObject() and
-      nodeFrom instanceof AllTarfileOpens and
-      nodeTo = call
-    )
+    nodeTo.(MethodCallNode).calls(nodeFrom, "getmembers") and
+    nodeFrom instanceof AllTarfileOpens
     or
-    exists(API::CallNode closing |
-      closing = API::moduleImport("contextlib").getMember("closing").getACall() and
-      nodeFrom = closing.getArg(0) and
-      nodeFrom = tarfileOpen().getReturn().getAValueReachingSink() and
-      nodeTo = closing
-    )
+    // To handle the case of `with closing(tarfile.open()) as file:`
+    // we add a step from the first argument of `closing` to the call to `closing`,
+    // whenever that first argument is a return of `tarfile.open()`.
+    nodeTo = API::moduleImport("contextlib").getMember("closing").getACall() and
+    nodeFrom = nodeTo.(API::CallNode).getArg(0) and
+    nodeFrom = tarfileOpen().getReturn().getAValueReachableFromSource()
   }
 }
 

python/ql/test/experimental/query-tests/Security/CWE-022/TarSlip.expected

@@ -18,6 +18,7 @@ edges
 | TarSlipImprov.py:133:7:133:39 | ControlFlowNode for Attribute() | TarSlipImprov.py:134:1:134:3 | ControlFlowNode for tar |
 | TarSlipImprov.py:141:6:141:29 | ControlFlowNode for Attribute() | 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:26:159:51 | ControlFlowNode for Attribute() | TarSlipImprov.py:169:9:169:12 | ControlFlowNode for tarc |
 | TarSlipImprov.py:176:6:176:31 | ControlFlowNode for Attribute() | TarSlipImprov.py:177:9:177:13 | GSSA Variable entry |
 | TarSlipImprov.py:177:9:177:13 | GSSA Variable entry | TarSlipImprov.py:178:36:178:40 | ControlFlowNode for entry |
 | TarSlipImprov.py:182:6:182:31 | ControlFlowNode for Attribute() | TarSlipImprov.py:183:9:183:13 | GSSA Variable entry |
@@ -71,6 +72,8 @@ nodes
 | TarSlipImprov.py:141:6:141:29 | ControlFlowNode for Attribute() | semmle.label | ControlFlowNode for Attribute() |
 | 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:26:159:51 | ControlFlowNode for Attribute() | semmle.label | ControlFlowNode for Attribute() |
+| TarSlipImprov.py:169:9:169:12 | ControlFlowNode for tarc | semmle.label | ControlFlowNode for tarc |
 | TarSlipImprov.py:176:6:176:31 | ControlFlowNode for Attribute() | semmle.label | ControlFlowNode for Attribute() |
 | TarSlipImprov.py:177:9:177:13 | GSSA Variable entry | semmle.label | GSSA Variable entry |
 | TarSlipImprov.py:178:36:178:40 | ControlFlowNode for entry | semmle.label | ControlFlowNode for entry |
@@ -124,6 +127,7 @@ subpaths
 | TarSlipImprov.py:130:5:130:7 | ControlFlowNode for tar | TarSlipImprov.py:129:6:129:26 | ControlFlowNode for Attribute() | TarSlipImprov.py:130:5:130:7 | ControlFlowNode for tar | Extraction of tarfile from $@ to a potentially untrusted source $@. | TarSlipImprov.py:129:6:129:26 | ControlFlowNode for Attribute() | ControlFlowNode for Attribute() | TarSlipImprov.py:130:5:130:7 | ControlFlowNode for tar | ControlFlowNode for tar |
 | TarSlipImprov.py:134:1:134:3 | ControlFlowNode for tar | TarSlipImprov.py:133:7:133:39 | ControlFlowNode for Attribute() | TarSlipImprov.py:134:1:134:3 | ControlFlowNode for tar | Extraction of tarfile from $@ to a potentially untrusted source $@. | TarSlipImprov.py:133:7:133:39 | ControlFlowNode for Attribute() | ControlFlowNode for Attribute() | TarSlipImprov.py:134:1:134:3 | ControlFlowNode for tar | ControlFlowNode for tar |
 | TarSlipImprov.py:143:36:143:40 | ControlFlowNode for entry | TarSlipImprov.py:141:6:141:29 | ControlFlowNode for Attribute() | TarSlipImprov.py:143:36:143:40 | ControlFlowNode for entry | Extraction of tarfile from $@ to a potentially untrusted source $@. | TarSlipImprov.py:141:6:141:29 | ControlFlowNode for Attribute() | ControlFlowNode for Attribute() | TarSlipImprov.py:143:36:143:40 | ControlFlowNode for entry | ControlFlowNode for entry |
+| TarSlipImprov.py:169:9:169:12 | ControlFlowNode for tarc | TarSlipImprov.py:159:26:159:51 | ControlFlowNode for Attribute() | TarSlipImprov.py:169:9:169:12 | ControlFlowNode for tarc | Extraction of tarfile from $@ to a potentially untrusted source $@. | TarSlipImprov.py:159:26:159:51 | ControlFlowNode for Attribute() | ControlFlowNode for Attribute() | TarSlipImprov.py:169:9:169:12 | ControlFlowNode for tarc | ControlFlowNode for tarc |
 | TarSlipImprov.py:178:36:178:40 | ControlFlowNode for entry | TarSlipImprov.py:176:6:176:31 | ControlFlowNode for Attribute() | TarSlipImprov.py:178:36:178:40 | ControlFlowNode for entry | Extraction of tarfile from $@ to a potentially untrusted source $@. | TarSlipImprov.py:176:6:176:31 | ControlFlowNode for Attribute() | ControlFlowNode for Attribute() | TarSlipImprov.py:178:36:178:40 | ControlFlowNode for entry | ControlFlowNode for entry |
 | TarSlipImprov.py:184:21:184:25 | ControlFlowNode for entry | TarSlipImprov.py:182:6:182:31 | ControlFlowNode for Attribute() | TarSlipImprov.py:184:21:184:25 | ControlFlowNode for entry | Extraction of tarfile from $@ to a potentially untrusted source $@. | TarSlipImprov.py:182:6:182:31 | ControlFlowNode for Attribute() | ControlFlowNode for Attribute() | TarSlipImprov.py:184:21:184:25 | ControlFlowNode for entry | ControlFlowNode for entry |
 | TarSlipImprov.py:189:1:189:3 | ControlFlowNode for tar | TarSlipImprov.py:188:7:188:27 | ControlFlowNode for Attribute() | TarSlipImprov.py:189:1:189:3 | ControlFlowNode for tar | Extraction of tarfile from $@ to a potentially untrusted source $@. | TarSlipImprov.py:188:7:188:27 | ControlFlowNode for Attribute() | ControlFlowNode for Attribute() | TarSlipImprov.py:189:1:189:3 | ControlFlowNode for tar | ControlFlowNode for tar |