Skip to content

Commit b81d41b

Browse files
joefarebrotherjoefarebrother
committed
Add django header write models for direct subscript write
1 parent 6538d22 commit b81d41b

File tree

5 files changed

+51
-3
lines changed

5 files changed

+51
-3
lines changed

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

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2274,6 +2274,36 @@ module PrivateDjango {
22742274

22752275
override predicate valueAllowsNewline() { none() }
22762276
}
2277+
2278+
/**
2279+
* A dict-like write to an item of an HTTP response, which is treated as a header write,
2280+
* such as `response[headerName] = value`
2281+
*/
2282+
class DjangoResponseSubscriptWrite extends Http::Server::ResponseHeaderWrite::Range {
2283+
DataFlow::Node index;
2284+
DataFlow::Node value;
2285+
2286+
DjangoResponseSubscriptWrite() {
2287+
exists(SubscriptNode subscript |
2288+
// To give `this` a value, we need to choose between either LHS or RHS,
2289+
// and just go with the LHS
2290+
this.asCfgNode() = subscript
2291+
|
2292+
subscript.getObject() =
2293+
DjangoImpl::DjangoHttp::Response::HttpResponse::instance().asCfgNode() and
2294+
value.asCfgNode() = subscript.(DefinitionNode).getValue() and
2295+
index.asCfgNode() = subscript.getIndex()
2296+
)
2297+
}
2298+
2299+
override DataFlow::Node getNameArg() { result = index }
2300+
2301+
override DataFlow::Node getValueArg() { result = value }
2302+
2303+
override predicate nameAllowsNewline() { none() }
2304+
2305+
override predicate valueAllowsNewline() { none() }
2306+
}
22772307
}
22782308
}
22792309

‎python/ql/test/experimental/query-tests/Security/CWE-614/CookieInjection.expected

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
11
edges
2+
| django_bad.py:27:33:27:67 | ControlFlowNode for Attribute() | django_bad.py:27:30:27:124 | ControlFlowNode for Fstring | provenance | |
3+
| django_bad.py:27:71:27:106 | ControlFlowNode for Attribute() | django_bad.py:27:30:27:124 | ControlFlowNode for Fstring | provenance | |
24
| flask_bad.py:1:26:1:32 | ControlFlowNode for ImportMember | flask_bad.py:1:26:1:32 | ControlFlowNode for request | provenance | |
35
| flask_bad.py:1:26:1:32 | ControlFlowNode for request | flask_bad.py:24:21:24:27 | ControlFlowNode for request | provenance | |
46
| flask_bad.py:1:26:1:32 | ControlFlowNode for request | flask_bad.py:24:49:24:55 | ControlFlowNode for request | provenance | |
@@ -12,6 +14,9 @@ edges
1214
nodes
1315
| django_bad.py:19:21:19:55 | ControlFlowNode for Attribute() | semmle.label | ControlFlowNode for Attribute() |
1416
| django_bad.py:20:21:20:56 | ControlFlowNode for Attribute() | semmle.label | ControlFlowNode for Attribute() |
17+
| django_bad.py:27:30:27:124 | ControlFlowNode for Fstring | semmle.label | ControlFlowNode for Fstring |
18+
| django_bad.py:27:33:27:67 | ControlFlowNode for Attribute() | semmle.label | ControlFlowNode for Attribute() |
19+
| django_bad.py:27:71:27:106 | ControlFlowNode for Attribute() | semmle.label | ControlFlowNode for Attribute() |
1520
| flask_bad.py:1:26:1:32 | ControlFlowNode for ImportMember | semmle.label | ControlFlowNode for ImportMember |
1621
| flask_bad.py:1:26:1:32 | ControlFlowNode for request | semmle.label | ControlFlowNode for request |
1722
| flask_bad.py:24:21:24:27 | ControlFlowNode for request | semmle.label | ControlFlowNode for request |
@@ -29,6 +34,12 @@ subpaths
2934
| django_bad.py:20:21:20:56 | ControlFlowNode for Attribute() | django_bad.py:20:21:20:56 | ControlFlowNode for Attribute() | django_bad.py:20:21:20:56 | ControlFlowNode for Attribute() | Cookie is constructed from a $@,and its httponly flag is not properly set. | django_bad.py:20:21:20:56 | ControlFlowNode for Attribute() | user-supplied input |
3035
| django_bad.py:20:21:20:56 | ControlFlowNode for Attribute() | django_bad.py:20:21:20:56 | ControlFlowNode for Attribute() | django_bad.py:20:21:20:56 | ControlFlowNode for Attribute() | Cookie is constructed from a $@,and its samesite flag is not properly set. | django_bad.py:20:21:20:56 | ControlFlowNode for Attribute() | user-supplied input |
3136
| django_bad.py:20:21:20:56 | ControlFlowNode for Attribute() | django_bad.py:20:21:20:56 | ControlFlowNode for Attribute() | django_bad.py:20:21:20:56 | ControlFlowNode for Attribute() | Cookie is constructed from a $@,and its secure flag is not properly set. | django_bad.py:20:21:20:56 | ControlFlowNode for Attribute() | user-supplied input |
37+
| django_bad.py:27:30:27:124 | ControlFlowNode for Fstring | django_bad.py:27:33:27:67 | ControlFlowNode for Attribute() | django_bad.py:27:30:27:124 | ControlFlowNode for Fstring | Cookie is constructed from a $@,and its httponly flag is not properly set. | django_bad.py:27:33:27:67 | ControlFlowNode for Attribute() | user-supplied input |
38+
| django_bad.py:27:30:27:124 | ControlFlowNode for Fstring | django_bad.py:27:33:27:67 | ControlFlowNode for Attribute() | django_bad.py:27:30:27:124 | ControlFlowNode for Fstring | Cookie is constructed from a $@,and its samesite flag is not properly set. | django_bad.py:27:33:27:67 | ControlFlowNode for Attribute() | user-supplied input |
39+
| django_bad.py:27:30:27:124 | ControlFlowNode for Fstring | django_bad.py:27:33:27:67 | ControlFlowNode for Attribute() | django_bad.py:27:30:27:124 | ControlFlowNode for Fstring | Cookie is constructed from a $@,and its secure flag is not properly set. | django_bad.py:27:33:27:67 | ControlFlowNode for Attribute() | user-supplied input |
40+
| django_bad.py:27:30:27:124 | ControlFlowNode for Fstring | django_bad.py:27:71:27:106 | ControlFlowNode for Attribute() | django_bad.py:27:30:27:124 | ControlFlowNode for Fstring | Cookie is constructed from a $@,and its httponly flag is not properly set. | django_bad.py:27:71:27:106 | ControlFlowNode for Attribute() | user-supplied input |
41+
| django_bad.py:27:30:27:124 | ControlFlowNode for Fstring | django_bad.py:27:71:27:106 | ControlFlowNode for Attribute() | django_bad.py:27:30:27:124 | ControlFlowNode for Fstring | Cookie is constructed from a $@,and its samesite flag is not properly set. | django_bad.py:27:71:27:106 | ControlFlowNode for Attribute() | user-supplied input |
42+
| django_bad.py:27:30:27:124 | ControlFlowNode for Fstring | django_bad.py:27:71:27:106 | ControlFlowNode for Attribute() | django_bad.py:27:30:27:124 | ControlFlowNode for Fstring | Cookie is constructed from a $@,and its secure flag is not properly set. | django_bad.py:27:71:27:106 | ControlFlowNode for Attribute() | user-supplied input |
3243
| flask_bad.py:24:21:24:40 | ControlFlowNode for Subscript | flask_bad.py:1:26:1:32 | ControlFlowNode for ImportMember | flask_bad.py:24:21:24:40 | ControlFlowNode for Subscript | Cookie is constructed from a $@,and its httponly flag is not properly set. | flask_bad.py:1:26:1:32 | ControlFlowNode for ImportMember | user-supplied input |
3344
| flask_bad.py:24:21:24:40 | ControlFlowNode for Subscript | flask_bad.py:1:26:1:32 | ControlFlowNode for ImportMember | flask_bad.py:24:21:24:40 | ControlFlowNode for Subscript | Cookie is constructed from a $@,and its samesite flag is not properly set. | flask_bad.py:1:26:1:32 | ControlFlowNode for ImportMember | user-supplied input |
3445
| flask_bad.py:24:21:24:40 | ControlFlowNode for Subscript | flask_bad.py:1:26:1:32 | ControlFlowNode for ImportMember | flask_bad.py:24:21:24:40 | ControlFlowNode for Subscript | Cookie is constructed from a $@,and its secure flag is not properly set. | flask_bad.py:1:26:1:32 | ControlFlowNode for ImportMember | user-supplied input |

‎python/ql/test/experimental/query-tests/Security/CWE-614/InsecureCookie.expected

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,15 @@
11
| django_bad.py:6:5:7:52 | ControlFlowNode for Attribute() | Cookie is added without the 'httponly' flag properly set. |
22
| django_bad.py:6:5:7:52 | ControlFlowNode for Attribute() | Cookie is added without the 'samesite' flag properly set. |
33
| django_bad.py:6:5:7:52 | ControlFlowNode for Attribute() | Cookie is added without the 'secure' flag properly set. |
4+
| django_bad.py:13:5:13:26 | ControlFlowNode for Subscript | Cookie is added without the 'httponly' flag properly set. |
5+
| django_bad.py:13:5:13:26 | ControlFlowNode for Subscript | Cookie is added without the 'samesite' flag properly set. |
6+
| django_bad.py:13:5:13:26 | ControlFlowNode for Subscript | Cookie is added without the 'secure' flag properly set. |
47
| django_bad.py:19:5:21:66 | ControlFlowNode for Attribute() | Cookie is added without the 'httponly' flag properly set. |
58
| django_bad.py:19:5:21:66 | ControlFlowNode for Attribute() | Cookie is added without the 'samesite' flag properly set. |
69
| django_bad.py:19:5:21:66 | ControlFlowNode for Attribute() | Cookie is added without the 'secure' flag properly set. |
10+
| django_bad.py:27:5:27:26 | ControlFlowNode for Subscript | Cookie is added without the 'httponly' flag properly set. |
11+
| django_bad.py:27:5:27:26 | ControlFlowNode for Subscript | Cookie is added without the 'samesite' flag properly set. |
12+
| django_bad.py:27:5:27:26 | ControlFlowNode for Subscript | Cookie is added without the 'secure' flag properly set. |
713
| django_good.py:19:5:19:44 | ControlFlowNode for Attribute() | Cookie is added without the 'httponly' flag properly set. |
814
| django_good.py:19:5:19:44 | ControlFlowNode for Attribute() | Cookie is added without the 'samesite' flag properly set. |
915
| django_good.py:19:5:19:44 | ControlFlowNode for Attribute() | Cookie is added without the 'secure' flag properly set. |

‎python/ql/test/experimental/query-tests/Security/CWE-614/django_bad.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ def django_response(request):
77
httponly=False, samesite='None')
88
return resp
99

10-
# This test no longer produces an output due to django header setting methods not being modeled in the main query pack
10+
1111
def django_response():
1212
response = django.http.HttpResponse()
1313
response['Set-Cookie'] = "name=value; SameSite=None;"
@@ -21,7 +21,7 @@ def django_response(request):
2121
secure=False, httponly=False, samesite='None')
2222
return resp
2323

24-
# This test no longer produces an output due to django header setting methods not being modeled in the main query pack
24+
2525
def django_response():
2626
response = django.http.HttpResponse()
2727
response['Set-Cookie'] = f"{django.http.request.GET.get('name')}={django.http.request.GET.get('value')}; SameSite=None;"

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ def redirect_through_normal_response(request):
6262

6363
resp = HttpResponse() # $ HttpResponse mimetype=text/html
6464
resp.status_code = 302
65-
resp['Location'] = next # $ MISSING: redirectLocation=next
65+
resp['Location'] = next # $ headerWriteName='Location' headerWriteValue=next MISSING: redirectLocation=next
6666
resp.content = private # $ MISSING: responseBody=private
6767
return resp
6868

@@ -134,4 +134,5 @@ def setting_cookie(request):
134134
resp.cookies["key3"] = "value3" # $ CookieWrite CookieName="key3" CookieValue="value3"
135135
resp.delete_cookie("key4") # $ CookieWrite CookieName="key4"
136136
resp.delete_cookie(key="key4") # $ CookieWrite CookieName="key4"
137+
resp["Set-Cookie"] = "key5=value5" # $ headerWriteName="Set-Cookie" headerWriteValue="key5=value5" CookieWrite CookieRawHeader="key5=value5"
137138
return resp

0 commit comments

Comments
 (0)