Skip to content

Commit 18c0d13

Browse files
committed
Python: Model most of UploadedFile in Django
1 parent 5ec5557 commit 18c0d13

File tree

2 files changed

+105
-10
lines changed

2 files changed

+105
-10
lines changed

python/ql/src/semmle/python/frameworks/Django.qll

Lines changed: 95 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -361,6 +361,67 @@ private module Django {
361361
}
362362
}
363363
}
364+
365+
/**
366+
* Provides models for the `django.core.files.uploadedfile.UploadedFile` class
367+
*
368+
* See https://docs.djangoproject.com/en/3.0/ref/files/uploads/#django.core.files.uploadedfile.UploadedFile.
369+
*/
370+
module UploadedFile {
371+
/** Gets a reference to the `django.core.files.uploadedfile.UploadedFile` class. */
372+
private API::Node classRef() {
373+
result =
374+
API::moduleImport("django")
375+
.getMember("core")
376+
.getMember("files")
377+
.getMember("uploadedfile")
378+
.getMember("UploadedFile")
379+
}
380+
381+
/**
382+
* A source of instances of `django.core.files.uploadedfile.UploadedFile`, extend this class to model new instances.
383+
*
384+
* This can include instantiations of the class, return values from function
385+
* calls, or a special parameter that will be set when functions are called by an external
386+
* library.
387+
*
388+
* Use the predicate `UploadedFile::instance()` to get references to instances of `django.core.files.uploadedfile.UploadedFile`.
389+
*/
390+
abstract class InstanceSource extends DataFlow::LocalSourceNode { }
391+
392+
/** A direct instantiation of `django.core.files.uploadedfile.UploadedFile`. */
393+
private class ClassInstantiation extends InstanceSource, DataFlow::CallCfgNode {
394+
override CallNode node;
395+
396+
ClassInstantiation() { this = classRef().getACall() }
397+
}
398+
399+
/** Gets a reference to an instance of `django.core.files.uploadedfile.UploadedFile`. */
400+
private DataFlow::TypeTrackingNode instance(DataFlow::TypeTracker t) {
401+
t.start() and
402+
result instanceof InstanceSource
403+
or
404+
exists(DataFlow::TypeTracker t2 | result = instance(t2).track(t2, t))
405+
}
406+
407+
/** Gets a reference to an instance of `django.core.files.uploadedfile.UploadedFile`. */
408+
DataFlow::Node instance() { instance(DataFlow::TypeTracker::end()).flowsTo(result) }
409+
410+
/**
411+
* Taint propagation for `django.core.files.uploadedfile.UploadedFile`.
412+
*/
413+
class UploadedFileAdditionalTaintStep extends TaintTracking::AdditionalTaintStep {
414+
override predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
415+
// Attributes
416+
nodeFrom = instance() and
417+
nodeTo.(DataFlow::AttrRead).getObject() = nodeFrom and
418+
nodeTo.(DataFlow::AttrRead).getAttributeName() in [
419+
"content_type", "content_type_extra", "content_type_extra", "charset", "name", "file"
420+
]
421+
// TODO: Model `file` with shared-filelike such that `request.FILES["key"].file.read()` works
422+
}
423+
}
424+
}
364425
}
365426

366427
/**
@@ -2002,7 +2063,6 @@ private module PrivateDjango {
20022063
// HttpHeaders (case insensitive dict-like)
20032064
"headers",
20042065
// MultiValueDict[str, UploadedFile]
2005-
// TODO: Model UploadedFile
20062066
"FILES",
20072067
// django.urls.ResolverMatch
20082068
// TODO: Model ResolverMatch
@@ -2020,6 +2080,40 @@ private module PrivateDjango {
20202080
}
20212081
}
20222082

2083+
/** An `UploadedFile` instance that originates from a django request. */
2084+
class DjangoHttpRequestUploadedFileInstances extends Django::UploadedFile::InstanceSource {
2085+
DjangoHttpRequestUploadedFileInstances() {
2086+
// TODO: this currently only works in local-scope, since writing type-trackers for
2087+
// this is a little too much effort. Once API-graphs are available for more
2088+
// things, we can rewrite this.
2089+
//
2090+
// TODO: This approach for identifying member-access is very adhoc, and we should
2091+
// be able to do something more structured for providing modeling of the members
2092+
// of a container-object.
2093+
//
2094+
// dicts
2095+
exists(DataFlow::AttrRead files, DataFlow::Node dict |
2096+
files.accesses(django::http::request::HttpRequest::instance(), "FILES") and
2097+
(
2098+
dict = files
2099+
or
2100+
dict.(DataFlow::MethodCallNode).calls(files, "dict")
2101+
)
2102+
|
2103+
this.asCfgNode().(SubscriptNode).getObject() = dict.asCfgNode()
2104+
or
2105+
this.(DataFlow::MethodCallNode).calls(dict, "get")
2106+
)
2107+
or
2108+
// getlist
2109+
exists(DataFlow::AttrRead files, DataFlow::MethodCallNode getlistCall |
2110+
files.accesses(django::http::request::HttpRequest::instance(), "FILES") and
2111+
getlistCall.calls(files, "getlist") and
2112+
this.asCfgNode().(SubscriptNode).getObject() = getlistCall.asCfgNode()
2113+
)
2114+
}
2115+
}
2116+
20232117
// ---------------------------------------------------------------------------
20242118
// django.shortcuts.redirect
20252119
// ---------------------------------------------------------------------------

python/ql/test/library-tests/frameworks/django-v2-v3/taint_test.py

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -61,22 +61,23 @@ def test_taint(request: HttpRequest, foo, bar, baz=None): # $requestHandler rou
6161
# MultiValueDict[str, UploadedFile]
6262
request.FILES, # $ tainted
6363
request.FILES["key"], # $ tainted
64-
request.FILES["key"].content_type, # $ MISSING: tainted
65-
request.FILES["key"].content_type_extra, # $ MISSING: tainted
66-
request.FILES["key"].content_type_extra["key"], # $ MISSING: tainted
67-
request.FILES["key"].charset, # $ MISSING: tainted
68-
request.FILES["key"].name, # $ MISSING: tainted
69-
request.FILES["key"].file, # $ MISSING: tainted
64+
request.FILES["key"].content_type, # $ tainted
65+
request.FILES["key"].content_type_extra, # $ tainted
66+
request.FILES["key"].content_type_extra["key"], # $ tainted
67+
request.FILES["key"].charset, # $ tainted
68+
request.FILES["key"].name, # $ tainted
69+
request.FILES["key"].file, # $ tainted
7070
request.FILES["key"].file.read(), # $ MISSING: tainted
7171

7272
request.FILES.get("key"), # $ tainted
73-
request.FILES.get("key").name, # $ MISSING: tainted
73+
request.FILES.get("key").name, # $ tainted
7474
request.FILES.getlist("key"), # $ tainted
7575
request.FILES.getlist("key")[0], # $ tainted
76-
request.FILES.getlist("key")[0].name, # $ MISSING: tainted
76+
request.FILES.getlist("key")[0].name, # $ tainted
7777
request.FILES.dict(), # $ tainted
7878
request.FILES.dict()["key"], # $ tainted
79-
request.FILES.dict()["key"].name, # $ MISSING: tainted
79+
request.FILES.dict()["key"].name, # $ tainted
80+
request.FILES.dict().get("key").name, # $ tainted
8081

8182
# Dict[str, Any]
8283
request.META, # $ tainted

0 commit comments

Comments
 (0)