Skip to content

Commit 7b5d9ec

Browse files
yoffyoff
committed
python: Straight port of tarslip
1 parent d900c3d commit 7b5d9ec

File tree

4 files changed

+179
-166
lines changed

4 files changed

+179
-166
lines changed

‎python/ql/lib/semmle/python/security/dataflow/TarSlip.qll‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
/**
2+
* Provides a taint-tracking configuration for detecting "command injection" vulnerabilities.
3+
*
4+
* Note, for performance reasons: only import this file if
5+
* `TarSlip::Configuration` is needed, otherwise
6+
* `TarSlipCustomizations` should be imported instead.
7+
*/
8+
9+
private import python
10+
import semmle.python.dataflow.new.DataFlow
11+
import semmle.python.dataflow.new.TaintTracking
12+
import TarSlipCustomizations::TarSlip
13+
14+
/**
15+
* A taint-tracking configuration for detecting "command injection" vulnerabilities.
16+
*/
17+
class Configuration extends TaintTracking::Configuration {
18+
Configuration() { this = "TarSlip" }
19+
20+
override predicate isSource(DataFlow::Node source) { source instanceof Source }
21+
22+
override predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
23+
24+
override predicate isSanitizer(DataFlow::Node node) { node instanceof Sanitizer }
25+
26+
override predicate isSanitizerGuard(DataFlow::BarrierGuard guard) {
27+
guard instanceof SanitizerGuard
28+
}
29+
}

‎python/ql/lib/semmle/python/security/dataflow/TarSlipCustomizations.qll‎

Lines changed: 145 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,145 @@
1+
/**
2+
* Provides default sources, sinks and sanitizers for detecting
3+
* "tar slip"
4+
* vulnerabilities, as well as extension points for adding your own.
5+
*/
6+
7+
private import python
8+
private import semmle.python.dataflow.new.DataFlow
9+
private import semmle.python.Concepts
10+
private import semmle.python.dataflow.new.BarrierGuards
11+
private import semmle.python.ApiGraphs
12+
13+
/**
14+
* Provides default sources, sinks and sanitizers for detecting
15+
* "tar slip"
16+
* vulnerabilities, as well as extension points for adding your own.
17+
*/
18+
module TarSlip {
19+
/**
20+
* A data flow source for "tar slip" vulnerabilities.
21+
*/
22+
abstract class Source extends DataFlow::Node { }
23+
24+
/**
25+
* A data flow sink for "tar slip" vulnerabilities.
26+
*/
27+
abstract class Sink extends DataFlow::Node { }
28+
29+
/**
30+
* A sanitizer for "tar slip" vulnerabilities.
31+
*/
32+
abstract class Sanitizer extends DataFlow::Node { }
33+
34+
/**
35+
* A sanitizer guard for "tar slip" vulnerabilities.
36+
*/
37+
abstract class SanitizerGuard extends DataFlow::BarrierGuard { }
38+
39+
/**
40+
* A source of exception info, considered as a flow source.
41+
*/
42+
class TarfileOpen extends Source {
43+
TarfileOpen() {
44+
this = API::moduleImport("tarfile").getMember("open").getACall() and
45+
// If argument refers to a string object, then it's a hardcoded path and
46+
// this tarfile is safe.
47+
not this.(DataFlow::CallCfgNode).getArg(0).getALocalSource().asExpr() instanceof StrConst and
48+
/* Ignore opens within the tarfile module itself */
49+
not this.getLocation().getFile().getBaseName() = "tarfile.py"
50+
}
51+
}
52+
53+
/**
54+
* For efficiency we don't want to track the flow of taint
55+
* around the tarfile module.
56+
*/
57+
class ExcludeTarFilePy extends Sanitizer {
58+
ExcludeTarFilePy() { this.getLocation().getFile().getBaseName() = "tarfile.py" }
59+
}
60+
61+
/**
62+
* For a call to `file.extractall` without arguments, `file` is considered a sink.
63+
*/
64+
class ExtractAllSink extends Sink {
65+
ExtractAllSink() {
66+
exists(DataFlow::CallCfgNode call |
67+
call =
68+
API::moduleImport("tarfile")
69+
.getMember("open")
70+
.getReturn()
71+
.getMember("extractall")
72+
.getACall() and
73+
not exists(call.getArg(_)) and
74+
not exists(call.getArgByName(_)) and
75+
this = call.(DataFlow::MethodCallNode).getObject()
76+
)
77+
}
78+
}
79+
80+
/**
81+
* An argument to `extract` is considered a sink.
82+
*/
83+
class ExtractSink extends Sink {
84+
ExtractSink() {
85+
exists(DataFlow::CallCfgNode call |
86+
call =
87+
API::moduleImport("tarfile").getMember("open").getReturn().getMember("extract").getACall() and
88+
this = call.getArg(0)
89+
)
90+
}
91+
}
92+
93+
/* Members argument to extract method */
94+
class ExtractMembersSink extends Sink {
95+
ExtractMembersSink() {
96+
exists(DataFlow::CallCfgNode call |
97+
call =
98+
API::moduleImport("tarfile")
99+
.getMember("open")
100+
.getReturn()
101+
.getMember("extractall")
102+
.getACall() and
103+
this in [call.getArg(0), call.getArgByName("members")]
104+
)
105+
}
106+
}
107+
108+
class TarFileInfoSanitizer extends SanitizerGuard {
109+
ControlFlowNode tarInfo;
110+
111+
TarFileInfoSanitizer() {
112+
exists(CallNode call, AttrNode attr |
113+
this = call and
114+
// We must test the name of the tar info object.
115+
attr = call.getAnArg() and
116+
attr.getName() = "name" and
117+
attr.getObject() = tarInfo
118+
|
119+
// Assume that any test with "path" in it is a sanitizer
120+
call.getAChild*().(AttrNode).getName().matches("%path")
121+
or
122+
call.getAChild*().(NameNode).getId().matches("%path")
123+
)
124+
}
125+
126+
override predicate checks(ControlFlowNode checked, boolean branch) {
127+
checked = tarInfo and
128+
branch in [true, false]
129+
}
130+
131+
DataFlow::ExprNode shouldGuard() {
132+
tarInfo.dominates(result.asCfgNode()) and
133+
// exists(EssaDefinition def |
134+
// def.getAUse() = tarInfo and
135+
// def.getAUse() = result.asCfgNode()
136+
// ) and
137+
exists(SsaSourceVariable v |
138+
v.getAUse() = tarInfo and
139+
v.getAUse() = result.asCfgNode()
140+
)
141+
}
142+
}
143+
144+
DataFlow::ExprNode getAGuardedNode(TarFileInfoSanitizer tfis) { result = tfis.getAGuardedNode() }
145+
}

‎python/ql/src/Security/CWE-022/TarSlip.ql‎

Lines changed: 5 additions & 165 deletions
Original file line numberDiff line numberDiff line change
@@ -13,170 +13,10 @@
1313
*/
1414

1515
import python
16-
import semmle.python.security.Paths
17-
import semmle.python.dataflow.TaintTracking
18-
import semmle.python.security.strings.Basic
16+
import semmle.python.security.dataflow.TarSlip
17+
import DataFlow::PathGraph
1918

20-
/** A TaintKind to represent open tarfile objects. That is, the result of calling `tarfile.open(...)` */
21-
class OpenTarFile extends TaintKind {
22-
OpenTarFile() { this = "tarfile.open" }
23-
24-
override TaintKind getTaintOfMethodResult(string name) {
25-
name = "getmember" and result instanceof TarFileInfo
26-
or
27-
name = "getmembers" and result.(SequenceKind).getItem() instanceof TarFileInfo
28-
}
29-
30-
override ClassValue getType() { result = Value::named("tarfile.TarFile") }
31-
32-
override TaintKind getTaintForIteration() { result instanceof TarFileInfo }
33-
}
34-
35-
/** The source of open tarfile objects. That is, any call to `tarfile.open(...)` */
36-
class TarfileOpen extends TaintSource {
37-
TarfileOpen() {
38-
Value::named("tarfile.open").getACall() = this and
39-
/*
40-
* If argument refers to a string object, then it's a hardcoded path and
41-
* this tarfile is safe.
42-
*/
43-
44-
not this.(CallNode).getAnArg().pointsTo(any(StringValue str)) and
45-
/* Ignore opens within the tarfile module itself */
46-
not this.(ControlFlowNode).getLocation().getFile().getBaseName() = "tarfile.py"
47-
}
48-
49-
override predicate isSourceOf(TaintKind kind) { kind instanceof OpenTarFile }
50-
}
51-
52-
class TarFileInfo extends TaintKind {
53-
TarFileInfo() { this = "tarfile.entry" }
54-
55-
override TaintKind getTaintOfMethodResult(string name) { name = "next" and result = this }
56-
57-
override TaintKind getTaintOfAttribute(string name) {
58-
name = "name" and result instanceof TarFileInfo
59-
}
60-
}
61-
62-
/*
63-
* For efficiency we don't want to track the flow of taint
64-
* around the tarfile module.
65-
*/
66-
67-
class ExcludeTarFilePy extends Sanitizer {
68-
ExcludeTarFilePy() { this = "Tar sanitizer" }
69-
70-
override predicate sanitizingNode(TaintKind taint, ControlFlowNode node) {
71-
node.getLocation().getFile().getBaseName() = "tarfile.py" and
72-
(
73-
taint instanceof OpenTarFile
74-
or
75-
taint instanceof TarFileInfo
76-
or
77-
taint.(SequenceKind).getItem() instanceof TarFileInfo
78-
)
79-
}
80-
}
81-
82-
/* Any call to an extractall method */
83-
class ExtractAllSink extends TaintSink {
84-
ExtractAllSink() {
85-
exists(CallNode call |
86-
this = call.getFunction().(AttrNode).getObject("extractall") and
87-
not exists(call.getAnArg())
88-
)
89-
}
90-
91-
override predicate sinks(TaintKind kind) { kind instanceof OpenTarFile }
92-
}
93-
94-
/* Argument to extract method */
95-
class ExtractSink extends TaintSink {
96-
CallNode call;
97-
98-
ExtractSink() {
99-
call.getFunction().(AttrNode).getName() = "extract" and
100-
this = call.getArg(0)
101-
}
102-
103-
override predicate sinks(TaintKind kind) { kind instanceof TarFileInfo }
104-
}
105-
106-
/* Members argument to extract method */
107-
class ExtractMembersSink extends TaintSink {
108-
CallNode call;
109-
110-
ExtractMembersSink() {
111-
call.getFunction().(AttrNode).getName() = "extractall" and
112-
(this = call.getArg(0) or this = call.getArgByName("members"))
113-
}
114-
115-
override predicate sinks(TaintKind kind) {
116-
kind.(SequenceKind).getItem() instanceof TarFileInfo
117-
or
118-
kind instanceof OpenTarFile
119-
}
120-
}
121-
122-
class TarFileInfoSanitizer extends Sanitizer {
123-
TarFileInfoSanitizer() { this = "TarInfo sanitizer" }
124-
125-
/* The test `if <path_sanitizing_test>:` clears taint on its `false` edge. */
126-
override predicate sanitizingEdge(TaintKind taint, PyEdgeRefinement test) {
127-
taint instanceof TarFileInfo and
128-
clears_taint_on_false_edge(test.getTest(), test.getSense())
129-
}
130-
131-
private predicate clears_taint_on_false_edge(ControlFlowNode test, boolean sense) {
132-
path_sanitizing_test(test) and
133-
sense = false
134-
or
135-
// handle `not` (also nested)
136-
test.(UnaryExprNode).getNode().getOp() instanceof Not and
137-
clears_taint_on_false_edge(test.(UnaryExprNode).getOperand(), sense.booleanNot())
138-
}
139-
}
140-
141-
private predicate path_sanitizing_test(ControlFlowNode test) {
142-
/* Assume that any test with "path" in it is a sanitizer */
143-
test.getAChild+().(AttrNode).getName().matches("%path")
144-
or
145-
test.getAChild+().(NameNode).getId().matches("%path")
146-
}
147-
148-
class TarSlipConfiguration extends TaintTracking::Configuration {
149-
TarSlipConfiguration() { this = "TarSlip configuration" }
150-
151-
override predicate isSource(TaintTracking::Source source) { source instanceof TarfileOpen }
152-
153-
override predicate isSink(TaintTracking::Sink sink) {
154-
sink instanceof ExtractSink or
155-
sink instanceof ExtractAllSink or
156-
sink instanceof ExtractMembersSink
157-
}
158-
159-
override predicate isSanitizer(Sanitizer sanitizer) {
160-
sanitizer instanceof TarFileInfoSanitizer
161-
or
162-
sanitizer instanceof ExcludeTarFilePy
163-
}
164-
165-
override predicate isBarrier(DataFlow::Node node) {
166-
// Avoid flow into the tarfile module
167-
exists(ParameterDefinition def |
168-
node.asVariable().getDefinition() = def
169-
or
170-
node.asCfgNode() = def.getDefiningNode()
171-
|
172-
def.getScope() = Value::named("tarfile.open").(CallableValue).getScope()
173-
or
174-
def.isSelf() and def.getScope().getEnclosingModule().getName() = "tarfile"
175-
)
176-
}
177-
}
178-
179-
from TarSlipConfiguration config, TaintedPathSource src, TaintedPathSink sink
180-
where config.hasFlowPath(src, sink)
181-
select sink.getSink(), src, sink, "Extraction of tarfile from $@", src.getSource(),
19+
from Configuration config, DataFlow::PathNode source, DataFlow::PathNode sink
20+
where config.hasFlowPath(source, sink)
21+
select sink.getNode(), source, sink, "Extraction of tarfile from $@", source.getNode(),
18222
"a potentially untrusted source"

‎python/ql/test/query-tests/Security/CWE-022-TarSlip/options‎

Lines changed: 0 additions & 1 deletion
This file was deleted.

0 commit comments

Comments
 (0)