Skip to content

Commit e327fdb

Browse files
committed
Python: Model flask View classes
1 parent 0b1cece commit e327fdb

File tree

2 files changed

+148
-8
lines changed

2 files changed

+148
-8
lines changed

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

Lines changed: 144 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ private module FlaskModel {
3434
* WARNING: Only holds for a few predefined attributes.
3535
*/
3636
private DataFlow::Node flask_attr(DataFlow::TypeTracker t, string attr_name) {
37-
attr_name in ["request", "make_response", "Response"] and
37+
attr_name in ["request", "make_response", "Response", "views"] and
3838
(
3939
t.start() and
4040
result = DataFlow::importNode("flask" + "." + attr_name)
@@ -187,6 +187,76 @@ private module FlaskModel {
187187
*/
188188
DataFlow::Node response_class() { result = response_class(DataFlow::TypeTracker::end()) }
189189
}
190+
191+
// -------------------------------------------------------------------------
192+
// flask.views
193+
// -------------------------------------------------------------------------
194+
/** Gets a reference to the `flask.views` module. */
195+
DataFlow::Node views() { result = flask_attr("views") }
196+
197+
/** Provides models for the `flask.views` module */
198+
module views {
199+
/**
200+
* Gets a reference to the attribute `attr_name` of the `flask.views` module.
201+
* WARNING: Only holds for a few predefined attributes.
202+
*/
203+
private DataFlow::Node views_attr(DataFlow::TypeTracker t, string attr_name) {
204+
attr_name in ["View", "MethodView"] and
205+
(
206+
t.start() and
207+
result = DataFlow::importNode("flask.views" + "." + attr_name)
208+
or
209+
t.startInAttr(attr_name) and
210+
result = views()
211+
)
212+
or
213+
// Due to bad performance when using normal setup with `views_attr(t2, attr_name).track(t2, t)`
214+
// we have inlined that code and forced a join
215+
exists(DataFlow::TypeTracker t2 |
216+
exists(DataFlow::StepSummary summary |
217+
views_attr_first_join(t2, attr_name, result, summary) and
218+
t = t2.append(summary)
219+
)
220+
)
221+
}
222+
223+
pragma[nomagic]
224+
private predicate views_attr_first_join(
225+
DataFlow::TypeTracker t2, string attr_name, DataFlow::Node res,
226+
DataFlow::StepSummary summary
227+
) {
228+
DataFlow::StepSummary::step(views_attr(t2, attr_name), res, summary)
229+
}
230+
231+
/**
232+
* Gets a reference to the attribute `attr_name` of the `flask.views` module.
233+
* WARNING: Only holds for a few predefined attributes.
234+
*/
235+
private DataFlow::Node views_attr(string attr_name) {
236+
result = views_attr(DataFlow::TypeTracker::end(), attr_name)
237+
}
238+
239+
/**
240+
* Provides models for the `flask.views.View` class and subclasses.
241+
*
242+
* See https://flask.palletsprojects.com/en/1.1.x/views/#basic-principle.
243+
*/
244+
module View {
245+
/** Gets a reference to the `flask.views.View` class or any subclass. */
246+
private DataFlow::Node subclassRef(DataFlow::TypeTracker t) {
247+
t.start() and
248+
result = views_attr(["View", "MethodView"])
249+
or
250+
// subclasses in project code
251+
result.asExpr().(ClassExpr).getABase() = subclassRef(t.continue()).asExpr()
252+
or
253+
exists(DataFlow::TypeTracker t2 | result = subclassRef(t2).track(t2, t))
254+
}
255+
256+
/** Gets a reference to the `flask.views.View` class or any subclass. */
257+
DataFlow::Node subclassRef() { result = subclassRef(DataFlow::TypeTracker::end()) }
258+
}
259+
}
190260
}
191261

192262
/**
@@ -261,6 +331,52 @@ private module FlaskModel {
261331
// ---------------------------------------------------------------------------
262332
// routing modeling
263333
// ---------------------------------------------------------------------------
334+
/** A flask View class defined in project code. */
335+
class FlaskViewClassDef extends Class {
336+
FlaskViewClassDef() { this.getABase() = flask::views::View::subclassRef().asExpr() }
337+
338+
/** Gets a function that could handle incoming requests, if any. */
339+
Function getARequestHandler() {
340+
// TODO: This doesn't handle attribute assignment. Should be OK, but analysis is not as complete as with
341+
// points-to and `.lookup`, which would handle `post = my_post_handler` inside class def
342+
result = this.getAMethod() and
343+
result.getName() = "dispatch_request"
344+
}
345+
346+
/** Gets a reference to this class. */
347+
private DataFlow::Node getARef(DataFlow::TypeTracker t) {
348+
t.start() and
349+
result.asExpr().(ClassExpr) = this.getParent()
350+
or
351+
exists(DataFlow::TypeTracker t2 | result = this.getARef(t2).track(t2, t))
352+
}
353+
354+
/** Gets a reference to this class. */
355+
DataFlow::Node getARef() { result = this.getARef(DataFlow::TypeTracker::end()) }
356+
357+
/** Gets a reference to the `as_view` classmethod of this class. */
358+
private DataFlow::Node asViewRef(DataFlow::TypeTracker t) {
359+
t.startInAttr("as_view") and
360+
result = this.getARef()
361+
or
362+
exists(DataFlow::TypeTracker t2 | result = this.asViewRef(t2).track(t2, t))
363+
}
364+
365+
/** Gets a reference to the `as_view` classmethod of this class. */
366+
DataFlow::Node asViewRef() { result = this.asViewRef(DataFlow::TypeTracker::end()) }
367+
368+
/** Gets a reference to the result of calling the `as_view` classmethod of this class. */
369+
private DataFlow::Node asViewResult(DataFlow::TypeTracker t) {
370+
t.start() and
371+
result.asCfgNode().(CallNode).getFunction() = this.asViewRef().asCfgNode()
372+
or
373+
exists(DataFlow::TypeTracker t2 | result = asViewResult(t2).track(t2, t))
374+
}
375+
376+
/** Gets a reference to the result of calling the `as_view` classmethod of this class. */
377+
DataFlow::Node asViewResult() { result = asViewResult(DataFlow::TypeTracker::end()) }
378+
}
379+
264380
private string werkzeug_rule_re() {
265381
// since flask uses werkzeug internally, we are using its routing rules from
266382
// https://github.com/pallets/werkzeug/blob/4dc8d6ab840d4b78cbd5789cef91b01e3bde01d5/src/werkzeug/routing.py#L138-L151
@@ -318,12 +434,36 @@ private module FlaskModel {
318434
result.asCfgNode() in [node.getArg(0), node.getArgByName("rule")]
319435
}
320436

437+
DataFlow::Node getViewArg() {
438+
result.asCfgNode() in [node.getArg(2), node.getArgByName("view_func")]
439+
}
440+
321441
override Function getARequestHandler() {
322-
exists(DataFlow::Node view_func_arg, DataFlow::LocalSourceNode func_src |
323-
view_func_arg.asCfgNode() in [node.getArg(2), node.getArgByName("view_func")] and
324-
func_src.flowsTo(view_func_arg) and
442+
exists(DataFlow::LocalSourceNode func_src |
443+
func_src.flowsTo(getViewArg()) and
325444
func_src.asExpr().(CallableExpr) = result.getDefinition()
326445
)
446+
or
447+
exists(FlaskViewClassDef vc |
448+
getViewArg() = vc.asViewResult() and
449+
result = vc.getARequestHandler()
450+
)
451+
}
452+
}
453+
454+
/** A request handler defined in a django view class, that has no known route. */
455+
private class FlaskViewClassHandlerWithoutKnownRoute extends HTTP::Server::RequestHandler::Range {
456+
FlaskViewClassHandlerWithoutKnownRoute() {
457+
exists(FlaskViewClassDef vc | vc.getARequestHandler() = this) and
458+
not exists(FlaskRouteSetup setup | setup.getARequestHandler() = this)
459+
}
460+
461+
override Parameter getARoutedParameter() {
462+
// Since we don't know the URL pattern, we simply mark all parameters as a routed
463+
// parameter. This should give us more RemoteFlowSources but could also lead to
464+
// more FPs. If this turns out to be the wrong tradeoff, we can always change our mind.
465+
result in [this.getArg(_), this.getArgByName(_)] and
466+
not result = this.getArg(0)
327467
}
328468
}
329469

python/ql/test/experimental/library-tests/frameworks/flask/routing_test.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,16 +37,16 @@ def unkown_route(foo, bar): # $requestHandler routedParameter=foo routedParamet
3737

3838
class ShowUser(View):
3939

40-
def dispatch_request(self, user_id): # $ MISSING: requestHandler routedParameter=user_id
41-
return "user_id: {}".format(user_id)
40+
def dispatch_request(self, user_id): # $ requestHandler routedParameter=user_id
41+
return "user_id: {}".format(user_id) # $ HttpResponse
4242

43-
app.add_url_rule("/basic/user/<int:user_id>", view_func=ShowUser.as_view('show_user')) # $routeSetup="/basic/user/<int:user_id>"
43+
app.add_url_rule("/basic/user/<int:user_id>", view_func=ShowUser.as_view('show_user')) # $routeSetup="/basic/user/<int:user_id>"
4444

4545

4646
class WithoutKnownRoute1(View):
4747
# For handler without known route, treat all parameters as routed parameters
4848
# (accepting that there might be a few FPs)
49-
def dispatch_request(self, foo, not_routed=42): # $ MISSING: requestHandler routedParameter=foo
49+
def dispatch_request(self, foo, not_routed=42): # $ requestHandler routedParameter=foo SPURIOUS: routedParameter=not_routed
5050
pass
5151

5252

0 commit comments

Comments
 (0)