Skip to content

Commit b3e53f8

Browse files
committed
Python: Model django.conf.urls.url (v 1.x)
1 parent be166d9 commit b3e53f8

File tree

5 files changed

+124
-50
lines changed

5 files changed

+124
-50
lines changed

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

Lines changed: 103 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ private module Django {
3434
* WARNING: Only holds for a few predefined attributes.
3535
*/
3636
private DataFlow::Node django_attr(DataFlow::TypeTracker t, string attr_name) {
37-
attr_name in ["db", "urls", "http"] and
37+
attr_name in ["db", "urls", "http", "conf"] and
3838
(
3939
t.start() and
4040
result = DataFlow::importNode("django" + "." + attr_name)
@@ -437,6 +437,55 @@ private module Django {
437437
DataFlow::Node re_path() { result = urls_attr("re_path") }
438438
}
439439

440+
// -------------------------------------------------------------------------
441+
// django.conf
442+
// -------------------------------------------------------------------------
443+
/** Gets a reference to the `django.conf` module. */
444+
DataFlow::Node conf() { result = django_attr("conf") }
445+
446+
/** Provides models for the `django.conf` module */
447+
module conf {
448+
// -------------------------------------------------------------------------
449+
// django.conf.urls
450+
// -------------------------------------------------------------------------
451+
/** Gets a reference to the `django.conf.urls` module. */
452+
private DataFlow::Node urls(DataFlow::TypeTracker t) {
453+
t.start() and
454+
result = DataFlow::importNode("django.conf.urls")
455+
or
456+
t.startInAttr("urls") and
457+
result = conf()
458+
or
459+
exists(DataFlow::TypeTracker t2 | result = urls(t2).track(t2, t))
460+
}
461+
462+
// NOTE: had to rename due to shadowing rules in QL
463+
/** Gets a reference to the `django.conf.urls` module. */
464+
DataFlow::Node conf_urls() { result = urls(DataFlow::TypeTracker::end()) }
465+
466+
// NOTE: had to rename due to shadowing rules in QL
467+
/** Provides models for the `django.conf.urls` module */
468+
module conf_urls {
469+
/** Gets a reference to the `django.conf.urls.url` function. */
470+
private DataFlow::Node url(DataFlow::TypeTracker t) {
471+
t.start() and
472+
result = DataFlow::importNode("django.conf.urls.url")
473+
or
474+
t.startInAttr("url") and
475+
result = conf_urls()
476+
or
477+
exists(DataFlow::TypeTracker t2 | result = url(t2).track(t2, t))
478+
}
479+
480+
/**
481+
* Gets a reference to the `django.conf.urls.url` function.
482+
*
483+
* See https://docs.djangoproject.com/en/1.11/ref/urls/#django.conf.urls.url
484+
*/
485+
DataFlow::Node url() { result = url(DataFlow::TypeTracker::end()) }
486+
}
487+
}
488+
440489
// -------------------------------------------------------------------------
441490
// django.http
442491
// -------------------------------------------------------------------------
@@ -684,28 +733,56 @@ private module Django {
684733
}
685734
}
686735

736+
/** A Django route setup that uses a Regex to specify route (and routed parameters). */
737+
abstract private class DjangoRegexRouteSetup extends DjangoRouteSetup {
738+
override Parameter getARoutedParameter() {
739+
// If we don't know the URL pattern, we simply mark all parameters as a routed
740+
// parameter. This should give us more RemoteFlowSources but could also lead to
741+
// more FPs. If this turns out to be the wrong tradeoff, we can always change our mind.
742+
exists(DjangoRouteHandler routeHandler | routeHandler = this.getARouteHandler() |
743+
not exists(this.getUrlPattern()) and
744+
result in [routeHandler.getArg(_), routeHandler.getArgByName(_)] and
745+
not result = any(int i | i <= routeHandler.getRequestParamIndex() | routeHandler.getArg(i))
746+
)
747+
or
748+
exists(DjangoRouteHandler routeHandler, DjangoRouteRegex regex |
749+
routeHandler = this.getARouteHandler() and
750+
regex.getRouteSetup() = this
751+
|
752+
// either using named capture groups (passed as keyword arguments) or using
753+
// unnamed capture groups (passed as positional arguments)
754+
not exists(regex.getGroupName(_, _)) and
755+
// first group will have group number 1
756+
result =
757+
routeHandler.getArg(routeHandler.getRequestParamIndex() + regex.getGroupNumber(_, _))
758+
or
759+
result = routeHandler.getArgByName(regex.getGroupName(_, _))
760+
)
761+
}
762+
}
763+
687764
/**
688-
* A regex that is used in a call to `django.urls.re_path`.
765+
* A regex that is used to set up a route.
689766
*
690767
* Needs this subclass to be considered a RegexString.
691768
*/
692-
private class DjangoUrlsRePathRegex extends RegexString {
693-
DjangoUrlsRePathCall rePathCall;
769+
private class DjangoRouteRegex extends RegexString {
770+
DjangoRegexRouteSetup rePathCall;
694771

695-
DjangoUrlsRePathRegex() {
772+
DjangoRouteRegex() {
696773
this instanceof StrConst and
697774
DataFlow::localFlow(DataFlow::exprNode(this), rePathCall.getUrlPatternArg())
698775
}
699776

700-
DjangoUrlsRePathCall getRePathCall() { result = rePathCall }
777+
DjangoRegexRouteSetup getRouteSetup() { result = rePathCall }
701778
}
702779

703780
/**
704781
* A call to `django.urls.re_path`.
705782
*
706783
* See https://docs.djangoproject.com/en/3.0/ref/urls/#re_path
707784
*/
708-
private class DjangoUrlsRePathCall extends DjangoRouteSetup {
785+
private class DjangoUrlsRePathCall extends DjangoRegexRouteSetup {
709786
override CallNode node;
710787

711788
DjangoUrlsRePathCall() { node.getFunction() = django::urls::re_path().asCfgNode() }
@@ -720,29 +797,26 @@ private module Django {
720797
djangoRouteHandlerFunctionTracker(result) = viewArg
721798
)
722799
}
800+
}
723801

724-
override Parameter getARoutedParameter() {
725-
// If we don't know the URL pattern, we simply mark all parameters as a routed
726-
// parameter. This should give us more RemoteFlowSources but could also lead to
727-
// more FPs. If this turns out to be the wrong tradeoff, we can always change our mind.
728-
exists(DjangoRouteHandler routeHandler | routeHandler = this.getARouteHandler() |
729-
not exists(this.getUrlPattern()) and
730-
result in [routeHandler.getArg(_), routeHandler.getArgByName(_)] and
731-
not result = any(int i | i <= routeHandler.getRequestParamIndex() | routeHandler.getArg(i))
732-
)
733-
or
734-
exists(DjangoRouteHandler routeHandler, DjangoUrlsRePathRegex regex |
735-
routeHandler = this.getARouteHandler() and
736-
regex.getRePathCall() = this
737-
|
738-
// either using named capture groups (passed as keyword arguments) or using
739-
// unnamed capture groups (passed as positional arguments)
740-
not exists(regex.getGroupName(_, _)) and
741-
// first group will have group number 1
742-
result =
743-
routeHandler.getArg(routeHandler.getRequestParamIndex() + regex.getGroupNumber(_, _))
744-
or
745-
result = routeHandler.getArgByName(regex.getGroupName(_, _))
802+
/**
803+
* A call to `django.conf.urls.url`.
804+
*
805+
* See https://docs.djangoproject.com/en/1.11/ref/urls/#django.conf.urls.url
806+
*/
807+
private class DjangoConfUrlsUrlCall extends DjangoRegexRouteSetup {
808+
override CallNode node;
809+
810+
DjangoConfUrlsUrlCall() { node.getFunction() = django::conf::conf_urls::url().asCfgNode() }
811+
812+
override DataFlow::Node getUrlPatternArg() {
813+
result.asCfgNode() = [node.getArg(0), node.getArgByName("regex")]
814+
}
815+
816+
override DjangoRouteHandler getARouteHandler() {
817+
exists(DataFlow::Node viewArg |
818+
viewArg.asCfgNode() in [node.getArg(1), node.getArgByName("view")] and
819+
djangoRouteHandlerFunctionTracker(result) = viewArg
746820
)
747821
}
748822
}

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

Lines changed: 17 additions & 17 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): # $f-:routeHandler $f-:routedParameter=foo $f-:routedParameter=bar
7+
def url_match_xss(request, foo, bar, no_taint=None): # $routeHandler $routedParameter=foo $routedParameter=bar
88
return HttpResponse('url_match_xss: {} {}'.format(foo, bar))
99

1010

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

1414

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

1818

19-
def http_resp_write(request): # $f-:routeHandler
19+
def http_resp_write(request): # $routeHandler
2020
rsp = HttpResponse()
2121
rsp.write(request.GET.get("untrusted"))
2222
return rsp
@@ -36,44 +36,44 @@ def get(self, request, untrusted): # $f-:routeHandler $f-:routedParameter=untru
3636
return HttpResponse('ClassView get: {}'.format(untrusted))
3737

3838

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

4343

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

4747

4848
urlpatterns = [
49-
url(r"^url_match/(?P<foo>[^/]+)/(?P<bar>[^/]+)", url_match_xss), # $f-:routeSetup="^url_match/(?P<foo>[^/]+)/(?P<bar>[^/]+)"
50-
url(r"^get_params", get_params_xss), # $f-:routeSetup="^get_params"
51-
url(r"^post_params", post_params_xss), # $f-:routeSetup="^post_params"
52-
url(r"^http_resp_write", http_resp_write), # $f-:routeSetup="^http_resp_write"
53-
url(r"^class_view/(?P<untrusted>.+)", ClassView.as_view()), # $f-:routeSetup="^class_view/(?P<untrusted>.+)"
49+
url(r"^url_match/(?P<foo>[^/]+)/(?P<bar>[^/]+)", url_match_xss), # $routeSetup="^url_match/(?P<foo>[^/]+)/(?P<bar>[^/]+)"
50+
url(r"^get_params", get_params_xss), # $routeSetup="^get_params"
51+
url(r"^post_params", post_params_xss), # $routeSetup="^post_params"
52+
url(r"^http_resp_write", http_resp_write), # $routeSetup="^http_resp_write"
53+
url(r"^class_view/(?P<untrusted>.+)", ClassView.as_view()), # $routeSetup="^class_view/(?P<untrusted>.+)"
5454

5555
# one pattern to support `articles/page-<n>` and ensuring that articles/ goes to page-1
56-
url(r"articles/^(?:page-(?P<page_number>\d+)/)?", show_articles), # $f-:routeSetup="articles/^(?:page-(?P<page_number>\d+)/)?"
56+
url(r"articles/^(?:page-(?P<page_number>\d+)/)?", show_articles), # $routeSetup="articles/^(?:page-(?P<page_number>\d+)/)?"
5757
# passing as positional argument is not the recommended way of doing things, but it is certainly
5858
# possible
59-
url(r"^([^/]+)/(?:foo|bar)/([^/]+)", xxs_positional_arg, name='xxs_positional_arg'), # $f-:routeSetup="^([^/]+)/(?:foo|bar)/([^/]+)"
59+
url(r"^([^/]+)/(?:foo|bar)/([^/]+)", xxs_positional_arg, name='xxs_positional_arg'), # $routeSetup="^([^/]+)/(?:foo|bar)/([^/]+)"
6060
]
6161

6262
################################################################################
6363
# Using patterns() for routing
6464

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

6868

69-
urlpatterns = patterns(url(r"^users/(?P<username>[^/]+)", show_user)) # $f-:routeSetup="^users/(?P<username>[^/]+)"
69+
urlpatterns = patterns(url(r"^users/(?P<username>[^/]+)", show_user)) # $routeSetup="^users/(?P<username>[^/]+)"
7070

7171
################################################################################
7272
# Show we understand the keyword arguments to django.conf.urls.url
7373

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

7777
urlpatterns = [
78-
url(view=kw_args, regex=r"^kw_args") # $f-:routeSetup="^kw_args"
78+
url(view=kw_args, regex=r"^kw_args") # $routeSetup="^kw_args"
7979
]

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -101,9 +101,9 @@ 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): # $f-:routeHandler
104+
def deprecated(request): # $routeHandler
105105
return HttpResponse('deprecated')
106106

107107
urlpatterns = [
108-
url(r"^deprecated/", deprecated), # $f-:routeSetup="^deprecated/"
108+
url(r"^deprecated/", deprecated), # $routeSetup="^deprecated/"
109109
]

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,5 +11,5 @@
1111
# inline expectation tests (which thinks the `$` would mark the beginning of a new
1212
# line)
1313
re_path(r"^ba[rz]/", views.bar_baz), # $routeSetup="^ba[rz]/"
14-
url(r"^deprecated/", views.deprecated), # $f-routeSetup="^deprecated/"
14+
url(r"^deprecated/", views.deprecated), # $routeSetup="^deprecated/"
1515
]

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,5 +6,5 @@ def foo(request: HttpRequest): # $routeHandler
66
def bar_baz(request: HttpRequest): # $routeHandler
77
return HttpResponse("bar_baz")
88

9-
def deprecated(request: HttpRequest): # $f-:routeHandler
9+
def deprecated(request: HttpRequest): # $routeHandler
1010
return HttpResponse("deprecated")

0 commit comments

Comments
 (0)