Skip to content

Commit 92ccb79

Browse files
tausbntausbn
authored
Merge pull request #4415 from RasmusWL/python-flask-routed-parameter
Python: Add support for routed parameters in flask
2 parents 61ecec7 + 74bd045 commit 92ccb79

File tree

10 files changed

+462
-118
lines changed

10 files changed

+462
-118
lines changed

‎python/ql/src/experimental/dataflow/RemoteFlowSources.qll‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ private import python
22
private import experimental.dataflow.DataFlow
33
// Need to import since frameworks can extend `RemoteFlowSource::Range`
44
private import experimental.semmle.python.Frameworks
5+
private import experimental.semmle.python.Concepts
56

67
/**
78
* A data flow source of remote user input.

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

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
import python
88
private import experimental.dataflow.DataFlow
99
private import experimental.semmle.python.Frameworks
10+
private import experimental.dataflow.RemoteFlowSources
1011

1112
/**
1213
* A data-flow node that executes an operating system command,
@@ -38,3 +39,62 @@ module SystemCommandExecution {
3839
abstract DataFlow::Node getCommand();
3940
}
4041
}
42+
43+
/** Provides classes for modeling HTTP-related APIs. */
44+
module HTTP {
45+
/** Provides classes for modeling HTTP servers. */
46+
module Server {
47+
/**
48+
* An data-flow node that sets up a route on a server.
49+
*
50+
* Extend this class to refine existing API models. If you want to model new APIs,
51+
* extend `RouteSetup::Range` instead.
52+
*/
53+
class RouteSetup extends DataFlow::Node {
54+
RouteSetup::Range range;
55+
56+
RouteSetup() { this = range }
57+
58+
/** Gets the URL pattern for this route, if it can be statically determined. */
59+
string getUrlPattern() { result = range.getUrlPattern() }
60+
61+
/** Gets a function that will handle incoming requests for this route, if any. */
62+
Function getARouteHandler() { result = range.getARouteHandler() }
63+
64+
/**
65+
* Gets a parameter that will receive parts of the url when handling incoming
66+
* requests for this route, if any. These automatically become a `RemoteFlowSource`.
67+
*/
68+
Parameter getARoutedParameter() { result = range.getARoutedParameter() }
69+
}
70+
71+
/** Provides a class for modeling new HTTP routing APIs. */
72+
module RouteSetup {
73+
/**
74+
* An data-flow node that sets up a route on a server.
75+
*
76+
* Extend this class to model new APIs. If you want to refine existing API models,
77+
* extend `RouteSetup` instead.
78+
*/
79+
abstract class Range extends DataFlow::Node {
80+
/** Gets the URL pattern for this route, if it can be statically determined. */
81+
abstract string getUrlPattern();
82+
83+
/** Gets a function that will handle incoming requests for this route, if any. */
84+
abstract Function getARouteHandler();
85+
86+
/**
87+
* Gets a parameter that will receive parts of the url when handling incoming
88+
* requests for this route, if any. These automatically become a `RemoteFlowSource`.
89+
*/
90+
abstract Parameter getARoutedParameter();
91+
}
92+
}
93+
94+
private class RoutedParameter extends RemoteFlowSource::Range, DataFlow::ParameterNode {
95+
RoutedParameter() { this.getParameter() = any(RouteSetup setup).getARoutedParameter() }
96+
97+
override string getSourceType() { result = "RoutedParameter" }
98+
}
99+
}
100+
}

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

Lines changed: 150 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
/**
2-
* Provides classes modeling security-relevant aspects of the `flask` package.
2+
* Provides classes modeling security-relevant aspects of the `flask` PyPI package.
3+
* See https://flask.palletsprojects.com/en/1.1.x/.
34
*/
45

56
private import python
@@ -11,6 +12,10 @@ private import experimental.semmle.python.frameworks.Werkzeug
1112

1213
// for old improved impl see
1314
// https://github.com/github/codeql/blob/9f95212e103c68d0c1dfa4b6f30fb5d53954ccef/python/ql/src/semmle/python/web/flask/Request.qll
15+
/**
16+
* Provides models for the `flask` PyPI package.
17+
* See https://flask.palletsprojects.com/en/1.1.x/.
18+
*/
1419
private module Flask {
1520
/** Gets a reference to the `flask` module. */
1621
DataFlow::Node flask(DataFlow::TypeTracker t) {
@@ -23,6 +28,7 @@ private module Flask {
2328
/** Gets a reference to the `flask` module. */
2429
DataFlow::Node flask() { result = flask(DataFlow::TypeTracker::end()) }
2530

31+
/** Provides models for the `flask` module. */
2632
module flask {
2733
/** Gets a reference to the `flask.request` object. */
2834
DataFlow::Node request(DataFlow::TypeTracker t) {
@@ -32,13 +38,154 @@ private module Flask {
3238
t.startInAttr("request") and
3339
result = flask()
3440
or
35-
exists(DataFlow::TypeTracker t2 | result = flask::request(t2).track(t2, t))
41+
exists(DataFlow::TypeTracker t2 | result = request(t2).track(t2, t))
3642
}
3743

3844
/** Gets a reference to the `flask.request` object. */
39-
DataFlow::Node request() { result = flask::request(DataFlow::TypeTracker::end()) }
45+
DataFlow::Node request() { result = request(DataFlow::TypeTracker::end()) }
46+
47+
/** Gets a reference to the `flask.Flask` class. */
48+
private DataFlow::Node classFlask(DataFlow::TypeTracker t) {
49+
t.start() and
50+
result = DataFlow::importNode("flask.Flask")
51+
or
52+
t.startInAttr("Flask") and
53+
result = flask()
54+
or
55+
exists(DataFlow::TypeTracker t2 | result = classFlask(t2).track(t2, t))
56+
}
57+
58+
/** Gets a reference to the `flask.Flask` class. */
59+
DataFlow::Node classFlask() { result = classFlask(DataFlow::TypeTracker::end()) }
60+
61+
/** Gets a reference to an instance of `flask.Flask` (a Flask application). */
62+
private DataFlow::Node app(DataFlow::TypeTracker t) {
63+
t.start() and
64+
result.asCfgNode().(CallNode).getFunction() = flask::classFlask().asCfgNode()
65+
or
66+
exists(DataFlow::TypeTracker t2 | result = app(t2).track(t2, t))
67+
}
68+
69+
/** Gets a reference to an instance of `flask.Flask` (a flask application). */
70+
DataFlow::Node app() { result = app(DataFlow::TypeTracker::end()) }
71+
}
72+
73+
// ---------------------------------------------------------------------------
74+
// routing modeling
75+
// ---------------------------------------------------------------------------
76+
/**
77+
* Gets a reference to the attribute `attr_name` of a flask application.
78+
* WARNING: Only holds for a few predefined attributes.
79+
*/
80+
private DataFlow::Node app_attr(DataFlow::TypeTracker t, string attr_name) {
81+
attr_name in ["route", "add_url_rule"] and
82+
t.startInAttr(attr_name) and
83+
result = flask::app()
84+
or
85+
// Due to bad performance when using normal setup with `app_attr(t2, attr_name).track(t2, t)`
86+
// we have inlined that code and forced a join
87+
exists(DataFlow::TypeTracker t2 |
88+
exists(DataFlow::StepSummary summary |
89+
app_attr_first_join(t2, attr_name, result, summary) and
90+
t = t2.append(summary)
91+
)
92+
)
93+
}
94+
95+
pragma[nomagic]
96+
private predicate app_attr_first_join(
97+
DataFlow::TypeTracker t2, string attr_name, DataFlow::Node res, DataFlow::StepSummary summary
98+
) {
99+
DataFlow::StepSummary::step(app_attr(t2, attr_name), res, summary)
100+
}
101+
102+
/**
103+
* Gets a reference to the attribute `attr_name` of a flask application.
104+
* WARNING: Only holds for a few predefined attributes.
105+
*/
106+
private DataFlow::Node app_attr(string attr_name) {
107+
result = app_attr(DataFlow::TypeTracker::end(), attr_name)
108+
}
109+
110+
private string werkzeug_rule_re() {
111+
// since flask uses werkzeug internally, we are using its routing rules from
112+
// https://github.com/pallets/werkzeug/blob/4dc8d6ab840d4b78cbd5789cef91b01e3bde01d5/src/werkzeug/routing.py#L138-L151
113+
result =
114+
"(?<static>[^<]*)<(?:(?<converter>[a-zA-Z_][a-zA-Z0-9_]*)(?:\\((?<args>.*?)\\))?\\:)?(?<variable>[a-zA-Z_][a-zA-Z0-9_]*)>"
115+
}
116+
117+
/** A route setup made by flask (sharing handling of URL patterns). */
118+
abstract private class FlaskRouteSetup extends HTTP::Server::RouteSetup::Range {
119+
override Parameter getARoutedParameter() {
120+
// If we don't know the URL pattern, we simply mark all parameters as a routed
121+
// parameter. This should give us more RemoteFlowSources but could also lead to
122+
// more FPs. If this turns out to be the wrong tradeoff, we can always change our mind.
123+
not exists(this.getUrlPattern()) and
124+
result = this.getARouteHandler().getArgByName(_)
125+
or
126+
exists(string name |
127+
result = this.getARouteHandler().getArgByName(name) and
128+
exists(string match |
129+
match = this.getUrlPattern().regexpFind(werkzeug_rule_re(), _, _) and
130+
name = match.regexpCapture(werkzeug_rule_re(), 4)
131+
)
132+
)
133+
}
134+
135+
/** Gets the argument used to pass in the URL pattern. */
136+
abstract DataFlow::Node getUrlPatternArg();
137+
138+
override string getUrlPattern() {
139+
exists(StrConst str |
140+
DataFlow::localFlow(DataFlow::exprNode(str), this.getUrlPatternArg()) and
141+
result = str.getText()
142+
)
143+
}
144+
}
145+
146+
/**
147+
* A call to `flask.Flask.route`.
148+
*
149+
* See https://flask.palletsprojects.com/en/1.1.x/api/#flask.Flask.route
150+
*/
151+
private class FlaskAppRouteCall extends FlaskRouteSetup, DataFlow::CfgNode {
152+
override CallNode node;
153+
154+
FlaskAppRouteCall() { node.getFunction() = app_attr("route").asCfgNode() }
155+
156+
override DataFlow::Node getUrlPatternArg() {
157+
result.asCfgNode() in [node.getArg(0), node.getArgByName("rule")]
158+
}
159+
160+
override Function getARouteHandler() { result.getADecorator().getAFlowNode() = node }
161+
}
162+
163+
/**
164+
* A call to `flask.Flask.add_url_rule`.
165+
*
166+
* See https://flask.palletsprojects.com/en/1.1.x/api/#flask.Flask.add_url_rule
167+
*/
168+
private class FlaskAppAddUrlRule extends FlaskRouteSetup, DataFlow::CfgNode {
169+
override CallNode node;
170+
171+
FlaskAppAddUrlRule() { node.getFunction() = app_attr("add_url_rule").asCfgNode() }
172+
173+
override DataFlow::Node getUrlPatternArg() {
174+
result.asCfgNode() in [node.getArg(0), node.getArgByName("rule")]
175+
}
176+
177+
override Function getARouteHandler() {
178+
exists(DataFlow::Node view_func_arg, DataFlow::Node func_src |
179+
view_func_arg.asCfgNode() in [node.getArg(2), node.getArgByName("view_func")] and
180+
DataFlow::localFlow(func_src, view_func_arg) and
181+
func_src.asExpr().(CallableExpr) = result.getDefinition()
182+
)
183+
}
40184
}
41185

186+
// ---------------------------------------------------------------------------
187+
// flask.Request taint modeling
188+
// ---------------------------------------------------------------------------
42189
// TODO: Do we even need this class? :|
43190
/**
44191
* A source of remote flow from a flask request.

‎python/ql/test/experimental/library-tests/frameworks/flask/ConceptsTest.expected‎

Whitespace-only changes.

‎python/ql/test/experimental/library-tests/frameworks/flask/ConceptsTest.ql‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
import python
2+
import experimental.meta.ConceptsTest

0 commit comments

Comments
 (0)