Merge branch 'main' into saritai/docs-update-12431

This commit is contained in:
Sarita Iyer
2023-11-30 17:40:25 -05:00
committed by GitHub
4 changed files with 103 additions and 5 deletions

View File

@@ -0,0 +1,5 @@
---
category: minorAnalysis
---
- Added support for tarfile extraction filters as defined in [PEP-706](https://peps.python.org/pep-0706). In particular, calls to `TarFile.extract`, and `TarFile.extractall` are no longer considered to be sinks for the `py/tarslip` query if a sufficiently safe filter is provided.

View File

@@ -55,10 +55,38 @@ module TarSlip {
ExcludeTarFilePy() { this.getLocation().getFile().getBaseName() = "tarfile.py" }
}
/**
* Holds if `call` has an unsafe extraction filter, either by default (as the default is unsafe),
* or by being set to an explicitly unsafe value, such as `"fully_trusted"`, or `None`.
*/
private predicate hasUnsafeFilter(API::CallNode call) {
call =
API::moduleImport("tarfile")
.getMember("open")
.getReturn()
.getMember(["extract", "extractall"])
.getACall() and
(
exists(Expr filterValue |
filterValue = call.getParameter(4, "filter").getAValueReachingSink().asExpr() and
(
filterValue.(StrConst).getText() = "fully_trusted"
or
filterValue instanceof None
)
)
or
not exists(call.getParameter(4, "filter"))
)
}
/**
* A sink capturing method calls to `extractall`.
*
* For a call to `file.extractall` without arguments, `file` is considered a sink.
* For a call to `file.extractall`, `file` is considered a sink if
*
* - there are no other arguments, or
* - there are other arguments (except `members`), and the extraction filter is unsafe.
*/
class ExtractAllSink extends Sink {
ExtractAllSink() {
@@ -69,8 +97,13 @@ module TarSlip {
.getReturn()
.getMember("extractall")
.getACall() and
not exists(call.getArg(_)) and
not exists(call.getArgByName(_)) and
(
not exists(call.getArg(_)) and
not exists(call.getArgByName(_))
or
hasUnsafeFilter(call)
) and
not exists(call.getArgByName("members")) and
this = call.(DataFlow::MethodCallNode).getObject()
)
}
@@ -84,7 +117,8 @@ module TarSlip {
exists(DataFlow::CallCfgNode call |
call =
API::moduleImport("tarfile").getMember("open").getReturn().getMember("extract").getACall() and
this = call.getArg(0)
this = call.getArg(0) and
hasUnsafeFilter(call)
)
}
}
@@ -99,7 +133,8 @@ module TarSlip {
.getReturn()
.getMember("extractall")
.getACall() and
this in [call.getArg(0), call.getArgByName("members")]
this in [call.getArg(0), call.getArgByName("members")] and
hasUnsafeFilter(call)
)
}
}

View File

@@ -12,6 +12,15 @@ edges
| tarslip.py:58:1:58:3 | GSSA Variable tar | tarslip.py:59:5:59:9 | GSSA Variable entry |
| tarslip.py:58:7:58:39 | ControlFlowNode for Attribute() | tarslip.py:58:1:58:3 | GSSA Variable tar |
| tarslip.py:59:5:59:9 | GSSA Variable entry | tarslip.py:61:21:61:25 | ControlFlowNode for entry |
| tarslip.py:90:1:90:3 | GSSA Variable tar | tarslip.py:91:1:91:3 | ControlFlowNode for tar |
| tarslip.py:90:7:90:39 | ControlFlowNode for Attribute() | tarslip.py:90:1:90:3 | GSSA Variable tar |
| tarslip.py:94:1:94:3 | GSSA Variable tar | tarslip.py:95:5:95:9 | GSSA Variable entry |
| tarslip.py:94:7:94:39 | ControlFlowNode for Attribute() | tarslip.py:94:1:94:3 | GSSA Variable tar |
| tarslip.py:95:5:95:9 | GSSA Variable entry | tarslip.py:96:17:96:21 | ControlFlowNode for entry |
| tarslip.py:109:1:109:3 | GSSA Variable tar | tarslip.py:110:1:110:3 | ControlFlowNode for tar |
| tarslip.py:109:7:109:39 | ControlFlowNode for Attribute() | tarslip.py:109:1:109:3 | GSSA Variable tar |
| tarslip.py:112:1:112:3 | GSSA Variable tar | tarslip.py:113:24:113:26 | ControlFlowNode for tar |
| tarslip.py:112:7:112:39 | ControlFlowNode for Attribute() | tarslip.py:112:1:112:3 | GSSA Variable tar |
nodes
| tarslip.py:14:1:14:3 | GSSA Variable tar | semmle.label | GSSA Variable tar |
| tarslip.py:14:7:14:39 | ControlFlowNode for Attribute() | semmle.label | ControlFlowNode for Attribute() |
@@ -31,6 +40,19 @@ nodes
| tarslip.py:58:7:58:39 | ControlFlowNode for Attribute() | semmle.label | ControlFlowNode for Attribute() |
| tarslip.py:59:5:59:9 | GSSA Variable entry | semmle.label | GSSA Variable entry |
| tarslip.py:61:21:61:25 | ControlFlowNode for entry | semmle.label | ControlFlowNode for entry |
| tarslip.py:90:1:90:3 | GSSA Variable tar | semmle.label | GSSA Variable tar |
| tarslip.py:90:7:90:39 | ControlFlowNode for Attribute() | semmle.label | ControlFlowNode for Attribute() |
| tarslip.py:91:1:91:3 | ControlFlowNode for tar | semmle.label | ControlFlowNode for tar |
| tarslip.py:94:1:94:3 | GSSA Variable tar | semmle.label | GSSA Variable tar |
| tarslip.py:94:7:94:39 | ControlFlowNode for Attribute() | semmle.label | ControlFlowNode for Attribute() |
| tarslip.py:95:5:95:9 | GSSA Variable entry | semmle.label | GSSA Variable entry |
| tarslip.py:96:17:96:21 | ControlFlowNode for entry | semmle.label | ControlFlowNode for entry |
| tarslip.py:109:1:109:3 | GSSA Variable tar | semmle.label | GSSA Variable tar |
| tarslip.py:109:7:109:39 | ControlFlowNode for Attribute() | semmle.label | ControlFlowNode for Attribute() |
| tarslip.py:110:1:110:3 | ControlFlowNode for tar | semmle.label | ControlFlowNode for tar |
| tarslip.py:112:1:112:3 | GSSA Variable tar | semmle.label | GSSA Variable tar |
| tarslip.py:112:7:112:39 | ControlFlowNode for Attribute() | semmle.label | ControlFlowNode for Attribute() |
| tarslip.py:113:24:113:26 | ControlFlowNode for tar | semmle.label | ControlFlowNode for tar |
subpaths
#select
| tarslip.py:15:1:15:3 | ControlFlowNode for tar | tarslip.py:14:7:14:39 | ControlFlowNode for Attribute() | tarslip.py:15:1:15:3 | ControlFlowNode for tar | This file extraction depends on a $@. | tarslip.py:14:7:14:39 | ControlFlowNode for Attribute() | potentially untrusted source |
@@ -38,3 +60,7 @@ subpaths
| tarslip.py:39:17:39:21 | ControlFlowNode for entry | tarslip.py:35:7:35:39 | ControlFlowNode for Attribute() | tarslip.py:39:17:39:21 | ControlFlowNode for entry | This file extraction depends on a $@. | tarslip.py:35:7:35:39 | ControlFlowNode for Attribute() | potentially untrusted source |
| tarslip.py:43:24:43:26 | ControlFlowNode for tar | tarslip.py:42:7:42:39 | ControlFlowNode for Attribute() | tarslip.py:43:24:43:26 | ControlFlowNode for tar | This file extraction depends on a $@. | tarslip.py:42:7:42:39 | ControlFlowNode for Attribute() | potentially untrusted source |
| tarslip.py:61:21:61:25 | ControlFlowNode for entry | tarslip.py:58:7:58:39 | ControlFlowNode for Attribute() | tarslip.py:61:21:61:25 | ControlFlowNode for entry | This file extraction depends on a $@. | tarslip.py:58:7:58:39 | ControlFlowNode for Attribute() | potentially untrusted source |
| tarslip.py:91:1:91:3 | ControlFlowNode for tar | tarslip.py:90:7:90:39 | ControlFlowNode for Attribute() | tarslip.py:91:1:91:3 | ControlFlowNode for tar | This file extraction depends on a $@. | tarslip.py:90:7:90:39 | ControlFlowNode for Attribute() | potentially untrusted source |
| tarslip.py:96:17:96:21 | ControlFlowNode for entry | tarslip.py:94:7:94:39 | ControlFlowNode for Attribute() | tarslip.py:96:17:96:21 | ControlFlowNode for entry | This file extraction depends on a $@. | tarslip.py:94:7:94:39 | ControlFlowNode for Attribute() | potentially untrusted source |
| tarslip.py:110:1:110:3 | ControlFlowNode for tar | tarslip.py:109:7:109:39 | ControlFlowNode for Attribute() | tarslip.py:110:1:110:3 | ControlFlowNode for tar | This file extraction depends on a $@. | tarslip.py:109:7:109:39 | ControlFlowNode for Attribute() | potentially untrusted source |
| tarslip.py:113:24:113:26 | ControlFlowNode for tar | tarslip.py:112:7:112:39 | ControlFlowNode for Attribute() | tarslip.py:113:24:113:26 | ControlFlowNode for tar | This file extraction depends on a $@. | tarslip.py:112:7:112:39 | ControlFlowNode for Attribute() | potentially untrusted source |

View File

@@ -82,3 +82,35 @@ tar = tarfile.open(unsafe_filename_tar)
for entry in tar:
if not os.path.isabs(entry.name):
tar.extract(entry, "/tmp/unpack/")
# Extraction filters
extraction_filter = "fully_trusted"
tar = tarfile.open(unsafe_filename_tar)
tar.extractall(filter=extraction_filter) # unsafe
tar.close()
tar = tarfile.open(unsafe_filename_tar)
for entry in tar:
tar.extract(entry, filter=extraction_filter) # unsafe
extraction_filter = "data"
tar = tarfile.open(unsafe_filename_tar)
tar.extractall(filter=extraction_filter) # safe
tar.close()
tar = tarfile.open(unsafe_filename_tar)
for entry in tar:
tar.extract(entry, filter=extraction_filter) # safe
extraction_filter = None
tar = tarfile.open(unsafe_filename_tar)
tar.extractall(filter=extraction_filter) # unsafe
tar = tarfile.open(unsafe_filename_tar)
tar.extractall(members=tar, filter=extraction_filter) # unsafe
tar = tarfile.open(unsafe_filename_tar)
tar.extractall(members=safemembers(tar), filter=extraction_filter) # safe -- we assume `safemembers` makes up for the unsafe filter