Skip to content

Commit 004ff38

Browse files
RasmusWLRasmusWL
committed
Python: Add separate RequestHandler concept
Since I really want to use our existing infrastructure to model that we can recognize something as a request handler without it having a route, we need this as a separate concept. All tests have been adjusted. The early modeling was based on flask, where all request-handling is based on handling requests from a specific route. But with the standard library handling and handlers without routes, the naming had to change.
1 parent a9bbe1d commit 004ff38

File tree

13 files changed

+152
-102
lines changed

13 files changed

+152
-102
lines changed

‎python/ql/src/semmle/python/Concepts.qll‎

Lines changed: 52 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -314,7 +314,7 @@ module HTTP {
314314
string getUrlPattern() { result = range.getUrlPattern() }
315315

316316
/** Gets a function that will handle incoming requests for this route, if any. */
317-
Function getARouteHandler() { result = range.getARouteHandler() }
317+
Function getARequestHandler() { result = range.getARequestHandler() }
318318

319319
/**
320320
* Gets a parameter that will receive parts of the url when handling incoming
@@ -344,7 +344,7 @@ module HTTP {
344344
}
345345

346346
/** Gets a function that will handle incoming requests for this route, if any. */
347-
abstract Function getARouteHandler();
347+
abstract Function getARequestHandler();
348348

349349
/**
350350
* Gets a parameter that will receive parts of the url when handling incoming
@@ -354,8 +354,57 @@ module HTTP {
354354
}
355355
}
356356

357+
/**
358+
* A function that will handle incoming HTTP requests.
359+
*
360+
* Extend this class to refine existing API models. If you want to model new APIs,
361+
* extend `RequestHandler::Range` instead.
362+
*/
363+
class RequestHandler extends Function {
364+
RequestHandler::Range range;
365+
366+
RequestHandler() { this = range }
367+
368+
/**
369+
* Gets a parameter that could receive parts of the url when handling incoming
370+
* requests, if any. These automatically become a `RemoteFlowSource`.
371+
*/
372+
Parameter getARoutedParameter() { result = range.getARoutedParameter() }
373+
}
374+
375+
/** Provides a class for modeling new HTTP request handlers. */
376+
module RequestHandler {
377+
/**
378+
* A function that will handle incoming HTTP requests.
379+
*
380+
* Extend this class to model new APIs. If you want to refine existing API models,
381+
* extend `RequestHandler` instead.
382+
*
383+
* Only extend this class if you can't provide a `RouteSetup`, since we handle that case automatically.
384+
*/
385+
abstract class Range extends Function {
386+
/**
387+
* Gets a parameter that could receive parts of the url when handling incoming
388+
* requests, if any. These automatically become a `RemoteFlowSource`.
389+
*/
390+
abstract Parameter getARoutedParameter();
391+
}
392+
}
393+
394+
private class RequestHandlerFromRouteSetup extends RequestHandler::Range {
395+
RouteSetup rs;
396+
397+
RequestHandlerFromRouteSetup() { this = rs.getARequestHandler() }
398+
399+
override Parameter getARoutedParameter() {
400+
result = rs.getARoutedParameter() and
401+
result in [this.getArg(_), this.getArgByName(_)]
402+
}
403+
}
404+
405+
/** A parameter that will receive parts of the url when handling an incoming request. */
357406
private class RoutedParameter extends RemoteFlowSource::Range, DataFlow::ParameterNode {
358-
RoutedParameter() { this.getParameter() = any(RouteSetup setup).getARoutedParameter() }
407+
RoutedParameter() { this.getParameter() = any(RequestHandler setup).getARoutedParameter() }
359408

360409
override string getSourceType() { result = "RoutedParameter" }
361410
}

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

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1676,7 +1676,7 @@ private module Django {
16761676
DjangoViewClassDef() { this.getABase() = django::views::generic::View::subclassRef().asExpr() }
16771677

16781678
/** Gets a function that could handle incoming requests, if any. */
1679-
DjangoRouteHandler getARouteHandler() {
1679+
DjangoRouteHandler getARequestHandler() {
16801680
// TODO: This doesn't handle attribute assignment. Should be OK, but analysis is not as complete as with
16811681
// points-to and `.lookup`, which would handle `post = my_post_handler` inside class def
16821682
result = this.getAMethod() and
@@ -1725,7 +1725,7 @@ private module Django {
17251725
DjangoRouteHandler() {
17261726
exists(djangoRouteHandlerFunctionTracker(this))
17271727
or
1728-
any(DjangoViewClassDef vc).getARouteHandler() = this
1728+
any(DjangoViewClassDef vc).getARequestHandler() = this
17291729
}
17301730

17311731
/** Gets the index of the request parameter. */
@@ -1746,12 +1746,12 @@ private module Django {
17461746
/** Gets the data-flow node that is used as the argument for the view handler. */
17471747
abstract DataFlow::Node getViewArg();
17481748

1749-
final override DjangoRouteHandler getARouteHandler() {
1749+
final override DjangoRouteHandler getARequestHandler() {
17501750
djangoRouteHandlerFunctionTracker(result) = getViewArg()
17511751
or
17521752
exists(DjangoViewClassDef vc |
17531753
getViewArg() = vc.asViewResult() and
1754-
result = vc.getARouteHandler()
1754+
result = vc.getARequestHandler()
17551755
)
17561756
}
17571757
}
@@ -1787,14 +1787,14 @@ private module Django {
17871787
// If we don't know the URL pattern, we simply mark all parameters as a routed
17881788
// parameter. This should give us more RemoteFlowSources but could also lead to
17891789
// more FPs. If this turns out to be the wrong tradeoff, we can always change our mind.
1790-
exists(DjangoRouteHandler routeHandler | routeHandler = this.getARouteHandler() |
1790+
exists(DjangoRouteHandler routeHandler | routeHandler = this.getARequestHandler() |
17911791
not exists(this.getUrlPattern()) and
17921792
result in [routeHandler.getArg(_), routeHandler.getArgByName(_)] and
17931793
not result = any(int i | i <= routeHandler.getRequestParamIndex() | routeHandler.getArg(i))
17941794
)
17951795
or
17961796
exists(string name |
1797-
result = this.getARouteHandler().getArgByName(name) and
1797+
result = this.getARequestHandler().getArgByName(name) and
17981798
exists(string match |
17991799
match = this.getUrlPattern().regexpFind(pathRoutedParameterRegex(), _, _) and
18001800
name = match.regexpCapture(pathRoutedParameterRegex(), 2)
@@ -1809,14 +1809,14 @@ private module Django {
18091809
// If we don't know the URL pattern, we simply mark all parameters as a routed
18101810
// parameter. This should give us more RemoteFlowSources but could also lead to
18111811
// more FPs. If this turns out to be the wrong tradeoff, we can always change our mind.
1812-
exists(DjangoRouteHandler routeHandler | routeHandler = this.getARouteHandler() |
1812+
exists(DjangoRouteHandler routeHandler | routeHandler = this.getARequestHandler() |
18131813
not exists(this.getUrlPattern()) and
18141814
result in [routeHandler.getArg(_), routeHandler.getArgByName(_)] and
18151815
not result = any(int i | i <= routeHandler.getRequestParamIndex() | routeHandler.getArg(i))
18161816
)
18171817
or
18181818
exists(DjangoRouteHandler routeHandler, DjangoRouteRegex regex |
1819-
routeHandler = this.getARouteHandler() and
1819+
routeHandler = this.getARequestHandler() and
18201820
regex.getRouteSetup() = this
18211821
|
18221822
// either using named capture groups (passed as keyword arguments) or using
@@ -1891,7 +1891,7 @@ private module Django {
18911891
class DjangoRouteHandlerRequestParam extends django::http::request::HttpRequest::InstanceSource,
18921892
RemoteFlowSource::Range, DataFlow::ParameterNode {
18931893
DjangoRouteHandlerRequestParam() {
1894-
this.getParameter() = any(DjangoRouteSetup setup).getARouteHandler().getRequestParam()
1894+
this.getParameter() = any(DjangoRouteSetup setup).getARequestHandler().getRequestParam()
18951895
}
18961896

18971897
override string getSourceType() { result = "django.http.request.HttpRequest" }

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -275,10 +275,10 @@ private module FlaskModel {
275275
// parameter. This should give us more RemoteFlowSources but could also lead to
276276
// more FPs. If this turns out to be the wrong tradeoff, we can always change our mind.
277277
not exists(this.getUrlPattern()) and
278-
result = this.getARouteHandler().getArgByName(_)
278+
result = this.getARequestHandler().getArgByName(_)
279279
or
280280
exists(string name |
281-
result = this.getARouteHandler().getArgByName(name) and
281+
result = this.getARequestHandler().getArgByName(name) and
282282
exists(string match |
283283
match = this.getUrlPattern().regexpFind(werkzeug_rule_re(), _, _) and
284284
name = match.regexpCapture(werkzeug_rule_re(), 4)
@@ -301,7 +301,7 @@ private module FlaskModel {
301301
result.asCfgNode() in [node.getArg(0), node.getArgByName("rule")]
302302
}
303303

304-
override Function getARouteHandler() { result.getADecorator().getAFlowNode() = node }
304+
override Function getARequestHandler() { result.getADecorator().getAFlowNode() = node }
305305
}
306306

307307
/**
@@ -318,7 +318,7 @@ private module FlaskModel {
318318
result.asCfgNode() in [node.getArg(0), node.getArgByName("rule")]
319319
}
320320

321-
override Function getARouteHandler() {
321+
override Function getARequestHandler() {
322322
exists(DataFlow::Node view_func_arg, DataFlow::Node func_src |
323323
view_func_arg.asCfgNode() in [node.getArg(2), node.getArgByName("view_func")] and
324324
DataFlow::localFlow(func_src, view_func_arg) and
@@ -484,7 +484,7 @@ private module FlaskModel {
484484
private class FlaskRouteHandlerReturn extends HTTP::Server::HttpResponse::Range, DataFlow::CfgNode {
485485
FlaskRouteHandlerReturn() {
486486
exists(Function routeHandler |
487-
routeHandler = any(FlaskRouteSetup rs).getARouteHandler() and
487+
routeHandler = any(FlaskRouteSetup rs).getARequestHandler() and
488488
node = routeHandler.getAReturnValueFlowNode()
489489
)
490490
}

‎python/ql/test/experimental/library-tests/frameworks/django-v1/routing_test.py‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,19 +4,19 @@
44
from django.views.generic import View
55

66

7-
def url_match_xss(request, foo, bar, no_taint=None): # $routeHandler routedParameter=foo routedParameter=bar
7+
def url_match_xss(request, foo, bar, no_taint=None): # $requestHandler routedParameter=foo routedParameter=bar
88
return HttpResponse('url_match_xss: {} {}'.format(foo, bar)) # $HttpResponse
99

1010

11-
def get_params_xss(request): # $routeHandler
11+
def get_params_xss(request): # $requestHandler
1212
return HttpResponse(request.GET.get("untrusted")) # $HttpResponse
1313

1414

15-
def post_params_xss(request): # $routeHandler
15+
def post_params_xss(request): # $requestHandler
1616
return HttpResponse(request.POST.get("untrusted")) # $HttpResponse
1717

1818

19-
def http_resp_write(request): # $routeHandler
19+
def http_resp_write(request): # $requestHandler
2020
rsp = HttpResponse() # $HttpResponse
2121
rsp.write(request.GET.get("untrusted")) # $HttpResponse
2222
return rsp
@@ -26,22 +26,22 @@ class Foo(object):
2626
# Note: since Foo is used as the super type in a class view, it will be able to handle requests.
2727

2828

29-
def post(self, request, untrusted): # $ MISSING: routeHandler routedParameter=untrusted
29+
def post(self, request, untrusted): # $ MISSING: requestHandler routedParameter=untrusted
3030
return HttpResponse('Foo post: {}'.format(untrusted)) # $HttpResponse
3131

3232

3333
class ClassView(View, Foo):
3434

35-
def get(self, request, untrusted): # $ routeHandler routedParameter=untrusted
35+
def get(self, request, untrusted): # $ requestHandler routedParameter=untrusted
3636
return HttpResponse('ClassView get: {}'.format(untrusted)) # $HttpResponse
3737

3838

39-
def show_articles(request, page_number=1): # $routeHandler routedParameter=page_number
39+
def show_articles(request, page_number=1): # $requestHandler routedParameter=page_number
4040
page_number = int(page_number)
4141
return HttpResponse('articles page: {}'.format(page_number)) # $HttpResponse
4242

4343

44-
def xxs_positional_arg(request, arg0, arg1, no_taint=None): # $routeHandler routedParameter=arg0 routedParameter=arg1
44+
def xxs_positional_arg(request, arg0, arg1, no_taint=None): # $requestHandler routedParameter=arg0 routedParameter=arg1
4545
return HttpResponse('xxs_positional_arg: {} {}'.format(arg0, arg1)) # $HttpResponse
4646

4747

@@ -62,7 +62,7 @@ def xxs_positional_arg(request, arg0, arg1, no_taint=None): # $routeHandler rou
6262
################################################################################
6363
# Using patterns() for routing
6464

65-
def show_user(request, username): # $routeHandler routedParameter=username
65+
def show_user(request, username): # $requestHandler routedParameter=username
6666
return HttpResponse('show_user {}'.format(username)) # $HttpResponse
6767

6868

@@ -71,7 +71,7 @@ def show_user(request, username): # $routeHandler routedParameter=username
7171
################################################################################
7272
# Show we understand the keyword arguments to django.conf.urls.url
7373

74-
def kw_args(request): # $routeHandler
74+
def kw_args(request): # $requestHandler
7575
return HttpResponse('kw_args') # $HttpResponse
7676

7777
urlpatterns = [

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

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -4,19 +4,19 @@
44
from django.views import View
55

66

7-
def url_match_xss(request, foo, bar, no_taint=None): # $routeHandler routedParameter=foo routedParameter=bar
7+
def url_match_xss(request, foo, bar, no_taint=None): # $requestHandler routedParameter=foo routedParameter=bar
88
return HttpResponse('url_match_xss: {} {}'.format(foo, bar)) # $HttpResponse
99

1010

11-
def get_params_xss(request): # $routeHandler
11+
def get_params_xss(request): # $requestHandler
1212
return HttpResponse(request.GET.get("untrusted")) # $HttpResponse
1313

1414

15-
def post_params_xss(request): # $routeHandler
15+
def post_params_xss(request): # $requestHandler
1616
return HttpResponse(request.POST.get("untrusted")) # $HttpResponse
1717

1818

19-
def http_resp_write(request): # $routeHandler
19+
def http_resp_write(request): # $requestHandler
2020
rsp = HttpResponse() # $HttpResponse
2121
rsp.write(request.GET.get("untrusted")) # $HttpResponse
2222
return rsp
@@ -26,22 +26,22 @@ class Foo(object):
2626
# Note: since Foo is used as the super type in a class view, it will be able to handle requests.
2727

2828

29-
def post(self, request, untrusted): # $ MISSING: routeHandler routedParameter=untrusted
29+
def post(self, request, untrusted): # $ MISSING: requestHandler routedParameter=untrusted
3030
return HttpResponse('Foo post: {}'.format(untrusted)) # $HttpResponse
3131

3232

3333
class ClassView(View, Foo):
3434

35-
def get(self, request, untrusted): # $ routeHandler routedParameter=untrusted
35+
def get(self, request, untrusted): # $ requestHandler routedParameter=untrusted
3636
return HttpResponse('ClassView get: {}'.format(untrusted)) # $HttpResponse
3737

3838

39-
def show_articles(request, page_number=1): # $routeHandler routedParameter=page_number
39+
def show_articles(request, page_number=1): # $requestHandler routedParameter=page_number
4040
page_number = int(page_number)
4141
return HttpResponse('articles page: {}'.format(page_number)) # $HttpResponse
4242

4343

44-
def xxs_positional_arg(request, arg0, arg1, no_taint=None): # $routeHandler routedParameter=arg0 routedParameter=arg1
44+
def xxs_positional_arg(request, arg0, arg1, no_taint=None): # $requestHandler routedParameter=arg0 routedParameter=arg1
4545
return HttpResponse('xxs_positional_arg: {} {}'.format(arg0, arg1)) # $HttpResponse
4646

4747

@@ -62,7 +62,7 @@ def xxs_positional_arg(request, arg0, arg1, no_taint=None): # $routeHandler rou
6262

6363
# Show we understand the keyword arguments to django.urls.re_path
6464

65-
def re_path_kwargs(request): # $routeHandler
65+
def re_path_kwargs(request): # $requestHandler
6666
return HttpResponse('re_path_kwargs') # $HttpResponse
6767

6868

@@ -75,16 +75,16 @@ def re_path_kwargs(request): # $routeHandler
7575
################################################################################
7676

7777
# saying page_number is an externally controlled *string* is a bit strange, when we have an int converter :O
78-
def page_number(request, page_number=1): # $routeHandler routedParameter=page_number
78+
def page_number(request, page_number=1): # $requestHandler routedParameter=page_number
7979
return HttpResponse('page_number: {}'.format(page_number)) # $HttpResponse
8080

81-
def foo_bar_baz(request, foo, bar, baz): # $routeHandler routedParameter=foo routedParameter=bar routedParameter=baz
81+
def foo_bar_baz(request, foo, bar, baz): # $requestHandler routedParameter=foo routedParameter=bar routedParameter=baz
8282
return HttpResponse('foo_bar_baz: {} {} {}'.format(foo, bar, baz)) # $HttpResponse
8383

84-
def path_kwargs(request, foo, bar): # $routeHandler routedParameter=foo routedParameter=bar
84+
def path_kwargs(request, foo, bar): # $requestHandler routedParameter=foo routedParameter=bar
8585
return HttpResponse('path_kwargs: {} {} {}'.format(foo, bar)) # $HttpResponse
8686

87-
def not_valid_identifier(request): # $routeHandler
87+
def not_valid_identifier(request): # $requestHandler
8888
return HttpResponse('<foo!>') # $HttpResponse
8989

9090
urlpatterns = [
@@ -101,7 +101,7 @@ def not_valid_identifier(request): # $routeHandler
101101
# This version 1.x way of defining urls is deprecated in Django 3.1, but still works
102102
from django.conf.urls import url
103103

104-
def deprecated(request): # $routeHandler
104+
def deprecated(request): # $requestHandler
105105
return HttpResponse('deprecated') # $HttpResponse
106106

107107
urlpatterns = [
@@ -113,5 +113,5 @@ class PossiblyNotRouted(View):
113113
# Even if our analysis can't find a route-setup for this class, we should still
114114
# consider it to be a handle incoming HTTP requests
115115

116-
def get(self, request, possibly_not_routed=42): # $ MISSING: routeHandler routedParameter=possibly_not_routed
116+
def get(self, request, possibly_not_routed=42): # $ MISSING: requestHandler routedParameter=possibly_not_routed
117117
return HttpResponse('PossiblyNotRouted get: {}'.format(possibly_not_routed)) # $HttpResponse

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
from django.http import HttpRequest
44

55

6-
def test_taint(request: HttpRequest, foo, bar, baz=None): # $routeHandler routedParameter=foo routedParameter=bar
6+
def test_taint(request: HttpRequest, foo, bar, baz=None): # $requestHandler routedParameter=foo routedParameter=bar
77
ensure_tainted(foo, bar)
88
ensure_not_tainted(baz)
99

0 commit comments

Comments
 (0)