hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-04-28 10:15:14 +02:00
Code Issues Packages Projects Releases Wiki Activity

Python: Add support for extraction filters

Browse Source 
Adds support for extraction filters as defined in
https://peps.python.org/pep-0706/
and implemented in Python 3.12.

By my reading, setting the filter to `'data'` or `'tar'` is probably
safe, whereas `'fully_trusted'` or the default (which is the same as
`None`) is not.

For now, I have just added this modelling to the tarslip query. We could
also share it with the modelling of `shutil.unpack_archive` (which has also
gained a `filter` argument), but it was unclear to me where we should put
this modelling in that case. Perhaps the best solution would be to merge
the experimental `py/tarslip-extended` query into the existing query (in
which case the current location is perhaps not too bad).
This commit is contained in:
Taus
2023-11-27 14:11:17 +00:00
parent f05c86239f
commit 95e9284d08
3 changed files with 108 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files

55
python/ql/lib/semmle/python/security/dataflow/TarSlipCustomizations.qll
View File

@@ -55,10 +55,48 @@ module TarSlip {
ExcludeTarFilePy() { this.getLocation().getFile().getBaseName() = "tarfile.py" }
}
private DataFlow::TypeTrackingNode unsafeFilter(DataFlow::TypeTracker t) {
t.start() and
(
result.asExpr().(StrConst).getS() = "fully_trusted"
or
result.asExpr() instanceof None
)
or
exists(DataFlow::TypeTracker t2 | result = unsafeFilter(t2).track(t2, t))
}
private DataFlow::Node unsafeFilter() {
unsafeFilter(DataFlow::TypeTracker::end()).flowsTo(result)
}
/**
* 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(DataFlow::CallCfgNode call) {
call =
API::moduleImport("tarfile")
.getMember("open")
.getReturn()
.getMember(["extract", "extractall"])
.getACall() and
(
call.getArg(4) = unsafeFilter()
or
call.getArgByName("filter") = unsafeFilter()
or
not exists(call.getArg(4)) and not exists(call.getArgByName("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 +107,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 +127,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 +143,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)
)
}
}