Skip to content

Commit ea4e263

Browse files
markshannonmarkshannon
committed
Python: Initial version and help of tar-slip (CWE-022) query.
1 parent cb43d27 commit ea4e263

File tree

7 files changed

+205
-0
lines changed

7 files changed

+205
-0
lines changed

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

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
6+
<overview>
7+
<p>Extracting files from a malicious tar archive without validating that the destination file path
8+
is within the destination directory can cause files outside the destination directory to be
9+
overwritten, due to the possible presence of directory traversal elements (<code>..</code>) in
10+
archive paths.</p>
11+
12+
<p>Tar archives contain archive entries representing each file in the archive. These entries
13+
include a file path for the entry, but these file paths are not restricted and may contain
14+
unexpected special elements such as the directory traversal element (<code>..</code>). If these
15+
file paths are used to determine an output file to write the contents of the archive item to, then
16+
the file may be written to an unexpected location. This can result in sensitive information being
17+
revealed or deleted, or an attacker being able to influence behavior by modifying unexpected
18+
files.</p>
19+
20+
<p>For example, if a tar archive contains a file entry <code>..\sneaky-file</code>, and the tar archive
21+
is extracted to the directory <code>c:\output</code>, then naively combining the paths would result
22+
in an output file path of <code>c:\output\..\sneaky-file</code>, which would cause the file to be
23+
written to <code>c:\sneaky-file</code>.</p>
24+
25+
</overview>
26+
<recommendation>
27+
28+
<p>Ensure that output paths constructed from tar archive entries are validated
29+
to prevent writing files to unexpected locations.</p>
30+
31+
<p>The recommended way of writing an output file from a tar archive entry is to check that
32+
<code>".."</code> does not occur in the path.
33+
</p>
34+
35+
</recommendation>
36+
37+
<example>
38+
<p>
39+
In this example an archive is extracted without validating file paths.
40+
If <code>archive.tar</code> contained relative paths (for
41+
instance, if it were created by something like <code>tar -cf archive.tar
42+
../file.txt</code>) then executing this code could write to locations
43+
outside the destination directory.
44+
</p>
45+
46+
<sample src="examples/tarslip_bad.py" />
47+
48+
<p>To fix this vulnerability, we need to check that the path does not
49+
contain any <code>".."</code> elements in it.
50+
</p>
51+
52+
<sample src="examples/tarslip_good.py" />
53+
54+
</example>
55+
<references>
56+
57+
<li>
58+
Snyk:
59+
<a href="https://snyk.io/research/zip-slip-vulnerability">Zip Slip Vulnerability</a>.
60+
</li>
61+
<li>
62+
OWASP:
63+
<a href="https://www.owasp.org/index.php/Path_traversal">Path Traversal</a>.
64+
</li>
65+
<li>
66+
Python Library Reference:
67+
< href="https://docs.python.org/3/library/tarfile.html#tarfile.TarFile.extract">TarFile.extract</a>.
68+
</li>
69+
<li>
70+
Python Library Reference:
71+
< href="https://docs.python.org/3/library/tarfile.html#tarfile.TarFile.extractall">TarFile.extractall</a>.
72+
</li>
73+
74+
</references>
75+
</qhelp>

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

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
/**
2+
* @name Arbitrary file write during tarfile extraction
3+
* @description Extracting files from a malicious tar archive without validating that the
4+
* destination file path is within the destination directory can cause files outside
5+
* the destination directory to be overwritten.
6+
* @kind path-problem
7+
* @id py/tarslip
8+
* @problem.severity error
9+
* @precision medium
10+
* @tags security
11+
* external/cwe/cwe-022
12+
*/
13+
14+
import python
15+
import semmle.python.security.Paths
16+
17+
import semmle.python.security.TaintTracking
18+
19+
/** A TaintKind to represent open tarfile objects. That is, the result of calling `tarfile.open(...)` */
20+
class OpenTarFile extends TaintKind {
21+
OpenTarFile() {
22+
this = "tarfile.open"
23+
}
24+
}
25+
26+
/** The source of open tarfile objects. That is, any call to `tarfile.open(...)` */
27+
class TarfileOpen extends TaintSource {
28+
29+
TarfileOpen() {
30+
Module::named("tarfile").attr("open").getACall() = this
31+
and
32+
/* If argument refers to a string object, then it's a hardcoded path and
33+
* this tarfile is safe.
34+
*/
35+
not this.(CallNode).getAnArg().refersTo(any(StringObject str))
36+
}
37+
38+
override predicate isSourceOf(TaintKind kind) {
39+
kind instanceof OpenTarFile
40+
}
41+
42+
}
43+
44+
/* Any call to an extract method */
45+
class ExtractionSink extends TaintSink {
46+
47+
CallNode call;
48+
49+
ExtractionSink() {
50+
this = call.getFunction().(AttrNode).getObject(extract())
51+
}
52+
53+
override predicate sinks(TaintKind kind) {
54+
kind instanceof OpenTarFile
55+
}
56+
57+
}
58+
59+
private string extract() {
60+
result = "extract" or result = "extractall"
61+
}
62+
63+
64+
//evil = [e for e in members if os.path.relpath(e).startswith(('/', '..'))]
65+
66+
class TarSlipConfiguration extends TaintTracking::Configuration {
67+
68+
TarSlipConfiguration() { this = "TarSlip configuration" }
69+
70+
override predicate isSource(TaintTracking::Source source) { source instanceof TarfileOpen }
71+
72+
override predicate isSink(TaintTracking::Sink sink) { sink instanceof ExtractionSink }
73+
74+
}
75+
76+
77+
from TarSlipConfiguration config, TaintedPathSource src, TaintedPathSink sink
78+
where config.hasFlowPath(src, sink)
79+
select sink.getSink(), src, sink, "Extraction of tarfile from $@", src.getSource(), "a potentially untrusted source"
80+
81+

‎python/ql/src/Security/CWE-022/examples/tarslip_bad.py‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
2+
import tarfile
3+
4+
with tarfile.open('archive.zip') as tar:
5+
#BAD : This could write any file on the filesystem.
6+
for entry in tar:
7+
tar.extract(entry, "/tmp/unpack/")

‎python/ql/src/Security/CWE-022/examples/tarslip_good.py‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
2+
import tarfile
3+
import os.path
4+
5+
with tarfile.open('archive.zip') as tar:
6+
for entry in tar:
7+
#GOOD: Check that entry is safe
8+
if os.path.isabs(entry.name) or ".." in entry.name:
9+
raise ValueError("Illegal tar archive entry")
10+
tar.extract(entry, "/tmp/unpack/")

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
edges
2+
| tarslip.py:12:7:12:39 | tarfile.open | tarslip.py:13:1:13:3 | tarfile.open |
3+
| tarslip.py:12:7:12:39 | tarfile.open | tarslip.py:14:1:14:3 | tarfile.open |
4+
| tarslip.py:16:7:16:39 | tarfile.open | tarslip.py:17:14:17:16 | tarfile.open |
5+
| tarslip.py:16:7:16:39 | tarfile.open | tarslip.py:18:5:18:7 | tarfile.open |
6+
parents
7+
#select
8+
| tarslip.py:13:1:13:3 | Taint sink | tarslip.py:12:7:12:39 | tarfile.open | tarslip.py:13:1:13:3 | tarfile.open | Extraction of tarfile from $@ | tarslip.py:12:7:12:39 | Taint source | a potentially untrusted source |
9+
| tarslip.py:18:5:18:7 | Taint sink | tarslip.py:16:7:16:39 | tarfile.open | tarslip.py:18:5:18:7 | tarfile.open | Extraction of tarfile from $@ | tarslip.py:16:7:16:39 | Taint source | a potentially untrusted source |

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Security/CWE-022/TarSlip.ql

‎python/ql/test/query-tests/Security/CWE-022/tarslip.py‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
#!/usr/bin/python
2+
import tarfile
3+
4+
unsafe_filename_tar = sys.argv[1]
5+
safe_filename_tar = "safe_path.tar"
6+
7+
8+
tar = tarfile.open(safe_filename_tar)
9+
for entry in tar:
10+
tar.extract(entry)
11+
12+
tar = tarfile.open(unsafe_filename_tar)
13+
tar.extractall()
14+
tar.close()
15+
16+
tar = tarfile.open(unsafe_filename_tar)
17+
for entry in tar:
18+
tar.extract(entry)
19+
20+
tar = tarfile.open(safe_filename_tar)
21+
tar.extractall()
22+
tar.close()

0 commit comments

Comments
 (0)