Skip to content

Commit 2b6e0cf

Browse files
RasmusWLRasmusWL
authored
Merge pull request #8340 from yoff/python/simple-csrf
python: minimal CSRF implementation
2 parents cb17e2a + aa3d7ba commit 2b6e0cf

File tree

10 files changed

+281
-2
lines changed

10 files changed

+281
-2
lines changed

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

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -969,6 +969,76 @@ module HTTP {
969969
abstract DataFlow::Node getValueArg();
970970
}
971971
}
972+
973+
/**
974+
* A data-flow node that enables or disables Cross-site request forgery protection
975+
* in a global manner.
976+
*
977+
* Extend this class to refine existing API models. If you want to model new APIs,
978+
* extend `CsrfProtectionSetting::Range` instead.
979+
*/
980+
class CsrfProtectionSetting extends DataFlow::Node instanceof CsrfProtectionSetting::Range {
981+
/**
982+
* Gets the boolean value corresponding to if CSRF protection is enabled
983+
* (`true`) or disabled (`false`) by this node.
984+
*/
985+
boolean getVerificationSetting() { result = super.getVerificationSetting() }
986+
}
987+
988+
/** Provides a class for modeling new CSRF protection setting APIs. */
989+
module CsrfProtectionSetting {
990+
/**
991+
* A data-flow node that enables or disables Cross-site request forgery protection
992+
* in a global manner.
993+
*
994+
* Extend this class to model new APIs. If you want to refine existing API models,
995+
* extend `CsrfProtectionSetting` instead.
996+
*/
997+
abstract class Range extends DataFlow::Node {
998+
/**
999+
* Gets the boolean value corresponding to if CSRF protection is enabled
1000+
* (`true`) or disabled (`false`) by this node.
1001+
*/
1002+
abstract boolean getVerificationSetting();
1003+
}
1004+
}
1005+
1006+
/**
1007+
* A data-flow node that enables or disables Cross-site request forgery protection
1008+
* for a specific part of an application.
1009+
*
1010+
* Extend this class to refine existing API models. If you want to model new APIs,
1011+
* extend `CsrfLocalProtectionSetting::Range` instead.
1012+
*/
1013+
class CsrfLocalProtectionSetting extends DataFlow::Node instanceof CsrfLocalProtectionSetting::Range {
1014+
/**
1015+
* Gets a request handler whose CSRF protection is changed.
1016+
*/
1017+
Function getRequestHandler() { result = super.getRequestHandler() }
1018+
1019+
/** Holds if CSRF protection is enabled by this setting */
1020+
predicate csrfEnabled() { super.csrfEnabled() }
1021+
}
1022+
1023+
/** Provides a class for modeling new CSRF protection setting APIs. */
1024+
module CsrfLocalProtectionSetting {
1025+
/**
1026+
* A data-flow node that enables or disables Cross-site request forgery protection
1027+
* for a specific part of an application.
1028+
*
1029+
* Extend this class to model new APIs. If you want to refine existing API models,
1030+
* extend `CsrfLocalProtectionSetting` instead.
1031+
*/
1032+
abstract class Range extends DataFlow::Node {
1033+
/**
1034+
* Gets a request handler whose CSRF protection is changed.
1035+
*/
1036+
abstract Function getRequestHandler();
1037+
1038+
/** Holds if CSRF protection is enabled by this setting */
1039+
abstract predicate csrfEnabled();
1040+
}
1041+
}
9721042
}
9731043

9741044
/** Provides classes for modeling HTTP clients. */

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

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2738,4 +2738,67 @@ module PrivateDjango {
27382738
.getAnImmediateUse()
27392739
}
27402740
}
2741+
2742+
// ---------------------------------------------------------------------------
2743+
// Settings
2744+
// ---------------------------------------------------------------------------
2745+
/**
2746+
* A custom middleware stack
2747+
*/
2748+
private class DjangoSettingsMiddlewareStack extends HTTP::Server::CsrfProtectionSetting::Range {
2749+
List list;
2750+
2751+
DjangoSettingsMiddlewareStack() {
2752+
this.asExpr() = list and
2753+
// we look for an assignment to the `MIDDLEWARE` setting
2754+
exists(DataFlow::Node mw |
2755+
mw.asVar().getName() = "MIDDLEWARE" and
2756+
DataFlow::localFlow(this, mw)
2757+
|
2758+
// To only include results where CSRF protection matters, we only care about CSRF
2759+
// protection when the django authentication middleware is enabled.
2760+
// Since an active session cookie is exactly what would allow an attacker to perform
2761+
// a CSRF attack.
2762+
// Notice that this does not ensure that this is not a FP, since the authentication
2763+
// middleware might be unused.
2764+
//
2765+
// This also strongly implies that `mw` is in fact a Django middleware setting and
2766+
// not just a variable named `MIDDLEWARE`.
2767+
list.getAnElt().(StrConst).getText() =
2768+
"django.contrib.auth.middleware.AuthenticationMiddleware"
2769+
)
2770+
}
2771+
2772+
override boolean getVerificationSetting() {
2773+
if
2774+
list.getAnElt().(StrConst).getText() in [
2775+
"django.middleware.csrf.CsrfViewMiddleware",
2776+
// see https://github.com/mozilla/django-session-csrf
2777+
"session_csrf.CsrfMiddleware"
2778+
]
2779+
then result = true
2780+
else result = false
2781+
}
2782+
}
2783+
2784+
private class DjangoCsrfDecorator extends HTTP::Server::CsrfLocalProtectionSetting::Range {
2785+
string decoratorName;
2786+
Function function;
2787+
2788+
DjangoCsrfDecorator() {
2789+
decoratorName in ["csrf_protect", "csrf_exempt", "requires_csrf_token", "ensure_csrf_cookie"] and
2790+
this =
2791+
API::moduleImport("django")
2792+
.getMember("views")
2793+
.getMember("decorators")
2794+
.getMember("csrf")
2795+
.getMember(decoratorName)
2796+
.getAUse() and
2797+
this.asExpr() = function.getADecorator()
2798+
}
2799+
2800+
override Function getRequestHandler() { result = function }
2801+
2802+
override predicate csrfEnabled() { decoratorName in ["csrf_protect", "requires_csrf_token"] }
2803+
}
27412804
}

‎python/ql/src/Security/CWE-352/CSRFProtectionDisabled.qhelp‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
6+
<overview>
7+
<p>
8+
Cross-site request forgery (CSRF) is a type of vulnerability in which an
9+
attacker is able to force a user to carry out an action that the user did
10+
not intend.
11+
</p>
12+
13+
<p>
14+
The attacker tricks an authenticated user into submitting a request to the
15+
web application. Typically this request will result in a state change on
16+
the server, such as changing the user's password. The request can be
17+
initiated when the user visits a site controlled by the attacker. If the
18+
web application relies only on cookies for authentication, or on other
19+
credentials that are automatically included in the request, then this
20+
request will appear as legitimate to the server.
21+
</p>
22+
23+
<p>
24+
A common countermeasure for CSRF is to generate a unique token to be
25+
included in the HTML sent from the server to a user. This token can be
26+
used as a hidden field to be sent back with requests to the server, where
27+
the server can then check that the token is valid and associated with the
28+
relevant user session.
29+
</p>
30+
</overview>
31+
32+
<recommendation>
33+
<p>
34+
In many web frameworks, CSRF protection is enabled by default. In these
35+
cases, using the default configuration is sufficient to guard against most
36+
CSRF attacks.
37+
</p>
38+
</recommendation>
39+
40+
<example>
41+
<p>
42+
The following example shows a case where CSRF protection is disabled by
43+
overriding the default middleware stack and not including the one protecting against CSRF.
44+
</p>
45+
46+
<sample src="examples/settings.py"/>
47+
48+
<p>
49+
The protecting middleware was probably commented out during a testing phase, when server-side token generation was not set up.
50+
Simply commenting it back in will enable CSRF protection.
51+
</p>
52+
53+
</example>
54+
55+
<references>
56+
<li>Wikipedia: <a href="https://en.wikipedia.org/wiki/Cross-site_request_forgery">Cross-site request forgery</a></li>
57+
<li>OWASP: <a href="https://owasp.org/www-community/attacks/csrf">Cross-site request forgery</a></li>
58+
</references>
59+
60+
</qhelp>

‎python/ql/src/Security/CWE-352/CSRFProtectionDisabled.ql‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
/**
2+
* @name CSRF protection weakened or disabled
3+
* @description Disabling or weakening CSRF protection may make the application
4+
* vulnerable to a Cross-Site Request Forgery (CSRF) attack.
5+
* @kind problem
6+
* @problem.severity warning
7+
* @security-severity 8.8
8+
* @precision high
9+
* @id py/csrf-protection-disabled
10+
* @tags security
11+
* external/cwe/cwe-352
12+
*/
13+
14+
import python
15+
import semmle.python.Concepts
16+
17+
predicate relevantSetting(HTTP::Server::CsrfProtectionSetting s) {
18+
// rule out test code as this is a common place to turn off CSRF protection.
19+
// We don't use normal `TestScope` to find test files, since we also want to match
20+
// a settings file such as `.../integration-tests/settings.py`
21+
not s.getLocation().getFile().getAbsolutePath().matches("%test%")
22+
}
23+
24+
predicate vulnerableSetting(HTTP::Server::CsrfProtectionSetting s) {
25+
s.getVerificationSetting() = false and
26+
not exists(HTTP::Server::CsrfLocalProtectionSetting p | p.csrfEnabled()) and
27+
relevantSetting(s)
28+
}
29+
30+
from HTTP::Server::CsrfProtectionSetting setting
31+
where
32+
vulnerableSetting(setting) and
33+
// We have seen examples of dummy projects with vulnerable settings alongside a main
34+
// project with a protecting settings file. We want to rule out this scenario, so we
35+
// require all non-test settings to be vulnerable.
36+
forall(HTTP::Server::CsrfProtectionSetting s | relevantSetting(s) | vulnerableSetting(s))
37+
select setting, "Potential CSRF vulnerability due to forgery protection being disabled or weakened."

‎python/ql/src/Security/CWE-352/examples/settings.py‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
MIDDLEWARE = [
2+
'django.middleware.security.SecurityMiddleware',
3+
'django.contrib.sessions.middleware.SessionMiddleware',
4+
'django.middleware.common.CommonMiddleware',
5+
# 'django.middleware.csrf.CsrfViewMiddleware',
6+
'django.contrib.auth.middleware.AuthenticationMiddleware',
7+
'django.contrib.messages.middleware.MessageMiddleware',
8+
'django.middleware.clickjacking.XFrameOptionsMiddleware',
9+
]

‎python/ql/src/change-notes/2022-03-24-csrf-protection.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: newQuery
3+
---
4+
* The query "CSRF protection weakened or disabled" (`py/csrf-protection-disabled`) has been implemented. Its results will now appear by default.

‎python/ql/test/experimental/meta/ConceptsTest.qll‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -540,6 +540,40 @@ class HttpClientRequestTest extends InlineExpectationsTest {
540540
}
541541
}
542542

543+
class CsrfProtectionSettingTest extends InlineExpectationsTest {
544+
CsrfProtectionSettingTest() { this = "CsrfProtectionSettingTest" }
545+
546+
override string getARelevantTag() { result = "CsrfProtectionSetting" }
547+
548+
override predicate hasActualResult(Location location, string element, string tag, string value) {
549+
exists(location.getFile().getRelativePath()) and
550+
exists(HTTP::Server::CsrfProtectionSetting setting |
551+
location = setting.getLocation() and
552+
element = setting.toString() and
553+
value = setting.getVerificationSetting().toString() and
554+
tag = "CsrfProtectionSetting"
555+
)
556+
}
557+
}
558+
559+
class CsrfLocalProtectionSettingTest extends InlineExpectationsTest {
560+
CsrfLocalProtectionSettingTest() { this = "CsrfLocalProtectionSettingTest" }
561+
562+
override string getARelevantTag() { result = "CsrfLocalProtection" + ["Enabled", "Disabled"] }
563+
564+
override predicate hasActualResult(Location location, string element, string tag, string value) {
565+
exists(location.getFile().getRelativePath()) and
566+
exists(HTTP::Server::CsrfLocalProtectionSetting p |
567+
location = p.getLocation() and
568+
element = p.toString() and
569+
value = p.getRequestHandler().getName().toString() and
570+
if p.csrfEnabled()
571+
then tag = "CsrfLocalProtectionEnabled"
572+
else tag = "CsrfLocalProtectionDisabled"
573+
)
574+
}
575+
}
576+
543577
class XmlParsingTest extends InlineExpectationsTest {
544578
XmlParsingTest() { this = "XmlParsingTest" }
545579

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
from django.http.response import HttpResponse, HttpResponseRedirect, HttpResponsePermanentRedirect, JsonResponse, HttpResponseNotFound
22
from django.views.generic import RedirectView
3+
from django.views.decorators.csrf import csrf_protect
34
import django.shortcuts
45
import json
56

@@ -117,6 +118,7 @@ class CustomJsonResponse(JsonResponse):
117118
def __init__(self, banner, content, *args, **kwargs):
118119
super().__init__(content, *args, content_type="text/html", **kwargs)
119120

121+
@csrf_protect # $CsrfLocalProtectionEnabled=safe__custom_json_response
120122
def safe__custom_json_response(request):
121123
return CustomJsonResponse("ACME Responses", {"foo": request.GET.get("foo")}) # $HttpResponse mimetype=application/json MISSING: responseBody=Dict SPURIOUS: responseBody="ACME Responses"
122124

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@
4040
'django.contrib.staticfiles',
4141
]
4242

43-
MIDDLEWARE = [
43+
MIDDLEWARE = [ # $CsrfProtectionSetting=false
4444
'django.middleware.security.SecurityMiddleware',
4545
'django.contrib.sessions.middleware.SessionMiddleware',
4646
'django.middleware.common.CommonMiddleware',

‎ruby/ql/src/queries/security/cwe-352/CSRFProtectionDisabled.qhelp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
<overview>
77
<p>
88
Cross-site request forgery (CSRF) is a type of vulnerability in which an
9-
attacker is able to force a user carry out an action that the user did
9+
attacker is able to force a user to carry out an action that the user did
1010
not intend.
1111
</p>
1212

0 commit comments

Comments
 (0)