Skip to content

Commit ff8708d

Browse files
committed
1 parent 8803fb2 commit ff8708d

File tree

3 files changed

+50
-13
lines changed

3 files changed

+50
-13
lines changed

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

Lines changed: 48 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,34 @@ private module Django {
160160
result = djangoRouteHandlerFunctionTracker(DataFlow::TypeTracker::end(), func)
161161
}
162162

163+
/**
164+
* A function that is used as a django route handler.
165+
*/
166+
private class DjangoRouteHandler extends Function {
167+
DjangoRouteHandler() { exists(djangoRouteHandlerFunctionTracker(this)) }
168+
169+
/** Gets the index of the request parameter. */
170+
int getRequestParamIndex() {
171+
not this.isMethod() and
172+
result = 0
173+
or
174+
this.isMethod() and
175+
result = 1
176+
}
177+
178+
/** Gets the request parameter. */
179+
Parameter getRequestParam() { result = this.getArg(this.getRequestParamIndex()) }
180+
}
181+
182+
/**
183+
* Gets the regex that is used by django to find routed parameters when using `django.urls.path`.
184+
*
185+
* Taken from https://github.com/django/django/blob/7d1bf29977bb368d7c28e7c6eb146db3b3009ae7/django/urls/resolvers.py#L199
186+
*/
187+
private string pathRoutedParameterRegex() {
188+
result = "<(?:(?<converter>[^>:]+):)?(?<parameter>\\w+)>"
189+
}
190+
163191
/**
164192
* A call to `django.urls.path`.
165193
*
@@ -174,14 +202,31 @@ private module Django {
174202
result.asCfgNode() = [node.getArg(0), node.getArgByName("route")]
175203
}
176204

177-
override Function getARouteHandler() {
205+
override DjangoRouteHandler getARouteHandler() {
178206
exists(DataFlow::Node viewArg |
179207
viewArg.asCfgNode() in [node.getArg(1), node.getArgByName("view")] and
180208
djangoRouteHandlerFunctionTracker(result) = viewArg
181209
)
182210
}
183211

184-
override Parameter getARoutedParameter() { none() }
212+
override Parameter getARoutedParameter() {
213+
// If we don't know the URL pattern, we simply mark all parameters as a routed
214+
// parameter. This should give us more RemoteFlowSources but could also lead to
215+
// more FPs. If this turns out to be the wrong tradeoff, we can always change our mind.
216+
exists(DjangoRouteHandler routeHandler | routeHandler = this.getARouteHandler() |
217+
not exists(this.getUrlPattern()) and
218+
result in [routeHandler.getArg(_), routeHandler.getArgByName(_)] and
219+
not result = any(int i | i <= routeHandler.getRequestParamIndex() | routeHandler.getArg(i))
220+
)
221+
or
222+
exists(string name |
223+
result = this.getARouteHandler().getArgByName(name) and
224+
exists(string match |
225+
match = this.getUrlPattern().regexpFind(pathRoutedParameterRegex(), _, _) and
226+
name = match.regexpCapture(pathRoutedParameterRegex(), 2)
227+
)
228+
)
229+
}
185230
}
186231

187232
/**
@@ -198,7 +243,7 @@ private module Django {
198243
result.asCfgNode() = [node.getArg(0), node.getArgByName("route")]
199244
}
200245

201-
override Function getARouteHandler() {
246+
override DjangoRouteHandler getARouteHandler() {
202247
exists(DataFlow::Node viewArg |
203248
viewArg.asCfgNode() in [node.getArg(1), node.getArgByName("view")] and
204249
djangoRouteHandlerFunctionTracker(result) = viewArg

python/ql/test/experimental/library-tests/frameworks/django-v2-v3/ConceptsTest.expected

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,5 @@
33
| routing_test.py:39:45:39:88 | Comment # $routeHandler $routedParameter=page_number | Missing result:routedParameter=page_number |
44
| routing_test.py:44:62:44:120 | Comment # $routeHandler $routedParameter=arg0 $routedParameter=arg1 | Missing result:routedParameter=arg0 |
55
| routing_test.py:44:62:44:120 | Comment # $routeHandler $routedParameter=arg0 $routedParameter=arg1 | Missing result:routedParameter=arg1 |
6-
| routing_test.py:78:43:78:86 | Comment # $routeHandler $routedParameter=page_number | Missing result:routedParameter=page_number |
7-
| routing_test.py:81:43:81:120 | Comment # $routeHandler $routedParameter=foo $routedParameter=bar $routedParameter=baz | Missing result:routedParameter=bar |
8-
| routing_test.py:81:43:81:120 | Comment # $routeHandler $routedParameter=foo $routedParameter=bar $routedParameter=baz | Missing result:routedParameter=baz |
9-
| routing_test.py:81:43:81:120 | Comment # $routeHandler $routedParameter=foo $routedParameter=bar $routedParameter=baz | Missing result:routedParameter=foo |
10-
| routing_test.py:84:38:84:94 | Comment # $routeHandler $routedParameter=foo $routedParameter=bar | Missing result:routedParameter=bar |
11-
| routing_test.py:84:38:84:94 | Comment # $routeHandler $routedParameter=foo $routedParameter=bar | Missing result:routedParameter=foo |
12-
| taint_test.py:6:60:6:116 | Comment # $routeHandler $routedParameter=foo $routedParameter=bar | Missing result:routedParameter=bar |
13-
| taint_test.py:6:60:6:116 | Comment # $routeHandler $routedParameter=foo $routedParameter=bar | Missing result:routedParameter=foo |
146
| testapp/views.py:3:33:3:47 | Comment # $routeHandler | Missing result:routeHandler= |
157
| testapp/views.py:6:37:6:51 | Comment # $routeHandler | Missing result:routeHandler= |

python/ql/test/experimental/library-tests/frameworks/django-v2-v3/TestTaint.expected

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
| taint_test.py:7 | fail | test_taint | bar |
2-
| taint_test.py:7 | fail | test_taint | foo |
1+
| taint_test.py:7 | ok | test_taint | bar |
2+
| taint_test.py:7 | ok | test_taint | foo |
33
| taint_test.py:8 | ok | test_taint | baz |
44
| taint_test.py:14 | fail | test_taint | request |
55
| taint_test.py:16 | fail | test_taint | request.body |

0 commit comments

Comments
 (0)