Skip to content

Commit 92a3846

Browse files
committed
Fix query to omit sinks within std lib files
1 parent fdbed2a commit 92a3846

File tree

1 file changed

+39
-33
lines changed

1 file changed

+39
-33
lines changed

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

Lines changed: 39 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ class AllTarfileOpens extends API::CallNode {
4949
)
5050
}
5151
}
52+
5253
/**
5354
* A taint-tracking configuration for detecting more "TarSlip" vulnerabilities.
5455
*/
@@ -58,39 +59,44 @@ class Configuration extends TaintTracking::Configuration {
5859
override predicate isSource(DataFlow::Node source) { source = tarfileOpen().getACall() }
5960

6061
override predicate isSink(DataFlow::Node sink) {
61-
// A sink capturing method calls to `extractall` without `members` argument.
62-
// For a call to `file.extractall` without `members` argument, `file` is considered a sink.
63-
exists(MethodCallNode call , AllTarfileOpens atfo|
64-
call = atfo.getReturn().getMember("extractall").getACall() and
65-
not exists(Node arg | arg = call.getArgByName("members")) and
66-
sink = call.getObject()
67-
)
68-
or
69-
// A sink capturing method calls to `extractall` with `members` argument.
70-
// For a call to `file.extractall` with `members` argument, `file` is considered a sink if not
71-
// a the `members` argument contains a NameConstant as None, a List or call to the method `getmembers`.
72-
// Otherwise, the argument of `members` is considered a sink.
73-
exists(MethodCallNode call, Node arg, AllTarfileOpens atfo|
74-
call = atfo.getReturn().getMember("extractall").getACall() and
75-
arg = call.getArgByName("members") and
76-
if
77-
arg.asCfgNode() instanceof NameConstantNode or
78-
arg.asCfgNode() instanceof ListNode
79-
then sink = call.getObject()
80-
else
81-
if arg.(MethodCallNode).getMethodName() = "getmembers"
82-
then sink = arg.(MethodCallNode).getObject()
83-
else sink = call.getArgByName("members")
84-
)
85-
or
86-
// An argument to `extract` is considered a sink.
87-
exists(AllTarfileOpens atfo | sink = atfo.getReturn().getMember("extract").getACall().getArg(0))
88-
or
89-
//An argument to `_extract_member` is considered a sink.
90-
exists(MethodCallNode call, AllTarfileOpens atfo |
91-
call = atfo.getReturn().getMember("_extract_member").getACall() and
92-
call.getArg(1).(AttrRead).accesses(sink, "name")
93-
)
62+
(
63+
// A sink capturing method calls to `extractall` without `members` argument.
64+
// For a call to `file.extractall` without `members` argument, `file` is considered a sink.
65+
exists(MethodCallNode call, AllTarfileOpens atfo |
66+
call = atfo.getReturn().getMember("extractall").getACall() and
67+
not exists(Node arg | arg = call.getArgByName("members")) and
68+
sink = call.getObject()
69+
)
70+
or
71+
// A sink capturing method calls to `extractall` with `members` argument.
72+
// For a call to `file.extractall` with `members` argument, `file` is considered a sink if not
73+
// a the `members` argument contains a NameConstant as None, a List or call to the method `getmembers`.
74+
// Otherwise, the argument of `members` is considered a sink.
75+
exists(MethodCallNode call, Node arg, AllTarfileOpens atfo |
76+
call = atfo.getReturn().getMember("extractall").getACall() and
77+
arg = call.getArgByName("members") and
78+
if
79+
arg.asCfgNode() instanceof NameConstantNode or
80+
arg.asCfgNode() instanceof ListNode
81+
then sink = call.getObject()
82+
else
83+
if arg.(MethodCallNode).getMethodName() = "getmembers"
84+
then sink = arg.(MethodCallNode).getObject()
85+
else sink = call.getArgByName("members")
86+
)
87+
or
88+
// An argument to `extract` is considered a sink.
89+
exists(AllTarfileOpens atfo |
90+
sink = atfo.getReturn().getMember("extract").getACall().getArg(0)
91+
)
92+
or
93+
//An argument to `_extract_member` is considered a sink.
94+
exists(MethodCallNode call, AllTarfileOpens atfo |
95+
call = atfo.getReturn().getMember("_extract_member").getACall() and
96+
call.getArg(1).(AttrRead).accesses(sink, "name")
97+
)
98+
) and
99+
not sink.getScope().getLocation().getFile().inStdlib()
94100
}
95101

96102
override predicate isAdditionalTaintStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {

0 commit comments

Comments
 (0)