Skip to content

Commit e8f548a

Browse files
committed
Python: Model routed parameter flow to *args and **kwargs in Django + rest framework
1 parent 24687b4 commit e8f548a

File tree

4 files changed

+29
-10
lines changed

4 files changed

+29
-10
lines changed

python/ql/lib/semmle/python/Concepts.qll

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -928,7 +928,10 @@ module Http {
928928

929929
override Parameter getARoutedParameter() {
930930
result = rs.getARoutedParameter() and
931-
result in [this.getArg(_), this.getArgByName(_)]
931+
result in [
932+
this.getArg(_), this.getArgByName(_), this.getVararg().(Parameter),
933+
this.getKwarg().(Parameter)
934+
]
932935
}
933936

934937
override string getFramework() { result = rs.getFramework() }

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

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2416,7 +2416,10 @@ module PrivateDjango {
24162416
// Since we don't know the URL pattern, we simply mark all parameters as a routed
24172417
// parameter. This should give us more RemoteFlowSources but could also lead to
24182418
// more FPs. If this turns out to be the wrong tradeoff, we can always change our mind.
2419-
result in [this.getArg(_), this.getArgByName(_)] and
2419+
result in [
2420+
this.getArg(_), this.getArgByName(_), //
2421+
this.getVararg().(Parameter), this.getKwarg().(Parameter), // TODO: These sources should be modeled as storing content!
2422+
] and
24202423
not result = any(int i | i < this.getFirstPossibleRoutedParamIndex() | this.getArg(i))
24212424
}
24222425

@@ -2452,13 +2455,20 @@ module PrivateDjango {
24522455
// more FPs. If this turns out to be the wrong tradeoff, we can always change our mind.
24532456
exists(DjangoRouteHandler routeHandler | routeHandler = this.getARequestHandler() |
24542457
not exists(this.getUrlPattern()) and
2455-
result in [routeHandler.getArg(_), routeHandler.getArgByName(_)] and
2458+
result in [
2459+
routeHandler.getArg(_), routeHandler.getArgByName(_), //
2460+
routeHandler.getVararg().(Parameter), routeHandler.getKwarg().(Parameter), // TODO: These sources should be modeled as storing content!
2461+
] and
24562462
not result =
24572463
any(int i | i < routeHandler.getFirstPossibleRoutedParamIndex() | routeHandler.getArg(i))
24582464
)
24592465
or
24602466
exists(string name |
2461-
result = this.getARequestHandler().getArgByName(name) and
2467+
(
2468+
result = this.getARequestHandler().getKwarg() // TODO: These sources should be modeled as storing content!
2469+
or
2470+
result = this.getARequestHandler().getArgByName(name)
2471+
) and
24622472
exists(string match |
24632473
match = this.getUrlPattern().regexpFind(pathRoutedParameterRegex(), _, _) and
24642474
name = match.regexpCapture(pathRoutedParameterRegex(), 2)
@@ -2475,7 +2485,10 @@ module PrivateDjango {
24752485
// more FPs. If this turns out to be the wrong tradeoff, we can always change our mind.
24762486
exists(DjangoRouteHandler routeHandler | routeHandler = this.getARequestHandler() |
24772487
not exists(this.getUrlPattern()) and
2478-
result in [routeHandler.getArg(_), routeHandler.getArgByName(_)] and
2488+
result in [
2489+
routeHandler.getArg(_), routeHandler.getArgByName(_), //
2490+
routeHandler.getVararg().(Parameter), routeHandler.getKwarg().(Parameter), // TODO: These sources should be modeled as storing content!
2491+
] and
24792492
not result =
24802493
any(int i | i < routeHandler.getFirstPossibleRoutedParamIndex() | routeHandler.getArg(i))
24812494
)

python/ql/lib/semmle/python/frameworks/RestFramework.qll

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,10 @@ private module RestFramework {
172172
// Since we don't know the URL pattern, we simply mark all parameters as a routed
173173
// parameter. This should give us more RemoteFlowSources but could also lead to
174174
// more FPs. If this turns out to be the wrong tradeoff, we can always change our mind.
175-
result in [this.getArg(_), this.getArgByName(_)] and
175+
result in [
176+
this.getArg(_), this.getArgByName(_), //
177+
this.getVararg().(Parameter), this.getKwarg().(Parameter), // TODO: These sources should be modeled as storing content!
178+
] and
176179
not result = any(int i | i < this.getFirstPossibleRoutedParamIndex() | this.getArg(i))
177180
}
178181

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -174,11 +174,11 @@ def some_method(self):
174174
)
175175

176176

177-
def kwargs_param(request, **kwargs): # $ requestHandler
177+
def kwargs_param(request, **kwargs): # $ requestHandler routedParameter=kwargs
178178
ensure_tainted(
179-
kwargs, # $ MISSING: tainted
180-
kwargs["foo"], # $ MISSING: tainted
181-
kwargs["bar"] # $ MISSING: tainted
179+
kwargs, # $ tainted
180+
kwargs["foo"], # $ tainted
181+
kwargs["bar"] # $ tainted
182182
)
183183

184184
ensure_tainted(request) # $ tainted

0 commit comments

Comments
 (0)