Skip to content

Commit ca0d345

Browse files
committed
Django: Model any class used in django route setup as view class
1 parent b428945 commit ca0d345

File tree

3 files changed

+38
-16
lines changed

3 files changed

+38
-16
lines changed
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,3 @@
11
lgtm,codescanning
22
* Improved modeling of `django` to recognize request handlers functions that are decorated (for example with `django.views.decorators.http.require_GET`). This leads to more sources of remote user input (`RemoteFlowSource`), since we correctly identify the first parameter as being passed a django request.
3+
* Improved modeling of django View classes. We now consider any class using in a routing setup with `<class>.as_view()` as django view class. This leads to more sources of remote user input (`RemoteFlowSource`), since we correctly identify the first parameter as being passed a django request.

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

Lines changed: 36 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2024,18 +2024,8 @@ private module Django {
20242024
result = djangoRouteHandlerFunctionTracker(DataFlow::TypeTracker::end(), func)
20252025
}
20262026

2027-
/** A django View class defined in project code. */
2028-
class DjangoViewClassDef extends Class {
2029-
DjangoViewClassDef() { this.getABase() = django::views::generic::View::subclassRef().asExpr() }
2030-
2031-
/** Gets a function that could handle incoming requests, if any. */
2032-
Function getARequestHandler() {
2033-
// TODO: This doesn't handle attribute assignment. Should be OK, but analysis is not as complete as with
2034-
// points-to and `.lookup`, which would handle `post = my_post_handler` inside class def
2035-
result = this.getAMethod() and
2036-
result.getName() = HTTP::httpVerbLower()
2037-
}
2038-
2027+
/** A class that might be a django View class. */
2028+
class PossibleDjangoViewClass extends Class {
20392029
/** Gets a reference to this class. */
20402030
private DataFlow::Node getARef(DataFlow::TypeTracker t) {
20412031
t.start() and
@@ -2070,6 +2060,37 @@ private module Django {
20702060
DataFlow::Node asViewResult() { result = asViewResult(DataFlow::TypeTracker::end()) }
20712061
}
20722062

2063+
/** A class that we consider a django View class. */
2064+
abstract class DjangoViewClass extends PossibleDjangoViewClass {
2065+
/** Gets a function that could handle incoming requests, if any. */
2066+
Function getARequestHandler() {
2067+
// TODO: This doesn't handle attribute assignment. Should be OK, but analysis is not as complete as with
2068+
// points-to and `.lookup`, which would handle `post = my_post_handler` inside class def
2069+
result = this.getAMethod() and
2070+
result.getName() = HTTP::httpVerbLower()
2071+
}
2072+
}
2073+
2074+
/**
2075+
* A class that is used in a route-setup, with `<class>.as_view()`, therefore being
2076+
* considered a django View class.
2077+
*/
2078+
class DjangoViewClassFromRouteSetup extends DjangoViewClass {
2079+
DjangoViewClassFromRouteSetup() {
2080+
exists(DjangoRouteSetup setup | setup.getViewArg() = this.asViewResult())
2081+
}
2082+
}
2083+
2084+
/**
2085+
* A class that has a super-type which is a django View class, therefore also
2086+
* becoming a django View class.
2087+
*/
2088+
class DjangoViewClassFromSuperClass extends DjangoViewClass {
2089+
DjangoViewClassFromSuperClass() {
2090+
this.getABase() = django::views::generic::View::subclassRef().asExpr()
2091+
}
2092+
}
2093+
20732094
/**
20742095
* A function that is a django route handler, meaning it handles incoming requests
20752096
* with the django framework.
@@ -2078,7 +2099,7 @@ private module Django {
20782099
DjangoRouteHandler() {
20792100
exists(DjangoRouteSetup route | route.getViewArg() = djangoRouteHandlerFunctionTracker(this))
20802101
or
2081-
any(DjangoViewClassDef vc).getARequestHandler() = this
2102+
any(DjangoViewClass vc).getARequestHandler() = this
20822103
}
20832104

20842105
/** Gets the index of the request parameter. */
@@ -2102,7 +2123,7 @@ private module Django {
21022123
final override DjangoRouteHandler getARequestHandler() {
21032124
djangoRouteHandlerFunctionTracker(result) = getViewArg()
21042125
or
2105-
exists(DjangoViewClassDef vc |
2126+
exists(DjangoViewClass vc |
21062127
getViewArg() = vc.asViewResult() and
21072128
result = vc.getARequestHandler()
21082129
)
@@ -2113,7 +2134,7 @@ private module Django {
21132134
private class DjangoViewClassHandlerWithoutKnownRoute extends HTTP::Server::RequestHandler::Range,
21142135
DjangoRouteHandler {
21152136
DjangoViewClassHandlerWithoutKnownRoute() {
2116-
exists(DjangoViewClassDef vc | vc.getARequestHandler() = this) and
2137+
exists(DjangoViewClass vc | vc.getARequestHandler() = this) and
21172138
not exists(DjangoRouteSetup setup | setup.getARequestHandler() = this)
21182139
}
21192140

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ def with_decorator(request, foo): # $ requestHandler routedParameter=foo
144144
class UnknownViewSubclass(UnknownViewSuperclass):
145145
# Although we don't know for certain that this class is a django view class, the fact that it's
146146
# used with `as_view()` in the routing setup should be enough that we treat it as such.
147-
def get(self, request): # $ MISSING: requestHandler
147+
def get(self, request): # $ requestHandler
148148
pass
149149

150150
urlpatterns = [

0 commit comments

Comments
 (0)