Skip to content

Commit 6681c18

Browse files
andrey-tyukin-sonarsourceandrey-tyukin-sonarsource
authored
SONARPY-668 Rule S4502: Disabling CSRF protection is security-sensitive (SonarSource#698)
1 parent 3a93aff commit 6681c18

17 files changed

Lines changed: 757 additions & 0 deletions

File tree

‎its/ruling/src/test/resources/expected/python-S4502.json‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
{
2+
'project:django-cms-3.7.1/cms/test_utils/project/sampleapp/views.py':[
3+
12,
4+
],
5+
}

‎python-checks/src/main/java/org/sonar/python/checks/CheckList.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import org.sonar.python.checks.hotspots.ClearTextProtocolsCheck;
2626
import org.sonar.python.checks.hotspots.CommandLineArgsCheck;
2727
import org.sonar.python.checks.hotspots.CorsCheck;
28+
import org.sonar.python.checks.hotspots.CsrfDisabledCheck;
2829
import org.sonar.python.checks.hotspots.DataEncryptionCheck;
2930
import org.sonar.python.checks.hotspots.DebugModeCheck;
3031
import org.sonar.python.checks.hotspots.DisabledHtmlAutoEscapeCheck;
@@ -81,6 +82,7 @@ public static Iterable<Class> getChecks() {
8182
ComparisonToNoneCheck.class,
8283
ConfusingWalrusCheck.class,
8384
CorsCheck.class,
85+
CsrfDisabledCheck.class,
8486
DataEncryptionCheck.class,
8587
DbNoPasswordCheck.class,
8688
DeadStoreCheck.class,

‎python-checks/src/main/java/org/sonar/python/checks/hotspots/CsrfDisabledCheck.java‎

Lines changed: 285 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,285 @@
1+
/*
2+
* SonarQube Python Plugin
3+
* Copyright (C) 2011-2020 SonarSource SA
4+
* mailto:info AT sonarsource DOT com
5+
*
6+
* This program is free software; you can redistribute it and/or
7+
* modify it under the terms of the GNU Lesser General Public
8+
* License as published by the Free Software Foundation; either
9+
* version 3 of the License, or (at your option) any later version.
10+
*
11+
* This program is distributed in the hope that it will be useful,
12+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
13+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
14+
* Lesser General Public License for more details.
15+
*
16+
* You should have received a copy of the GNU Lesser General Public License
17+
* along with this program; if not, write to the Free Software Foundation,
18+
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
19+
*/
20+
package org.sonar.python.checks.hotspots;
21+
22+
import java.util.Arrays;
23+
import java.util.HashSet;
24+
import java.util.List;
25+
import java.util.Locale;
26+
import java.util.Optional;
27+
import java.util.Set;
28+
import java.util.function.Predicate;
29+
import java.util.stream.Collectors;
30+
import org.sonar.check.Rule;
31+
import org.sonar.plugins.python.api.PythonSubscriptionCheck;
32+
import org.sonar.plugins.python.api.SubscriptionContext;
33+
import org.sonar.plugins.python.api.symbols.ClassSymbol;
34+
import org.sonar.plugins.python.api.symbols.Symbol;
35+
import org.sonar.plugins.python.api.symbols.Usage;
36+
import org.sonar.plugins.python.api.tree.AssignmentStatement;
37+
import org.sonar.plugins.python.api.tree.CallExpression;
38+
import org.sonar.plugins.python.api.tree.ClassDef;
39+
import org.sonar.plugins.python.api.tree.Decorator;
40+
import org.sonar.plugins.python.api.tree.DictionaryLiteral;
41+
import org.sonar.plugins.python.api.tree.Expression;
42+
import org.sonar.plugins.python.api.tree.KeyValuePair;
43+
import org.sonar.plugins.python.api.tree.ListLiteral;
44+
import org.sonar.plugins.python.api.tree.Name;
45+
import org.sonar.plugins.python.api.tree.RegularArgument;
46+
import org.sonar.plugins.python.api.tree.StringLiteral;
47+
import org.sonar.plugins.python.api.tree.SubscriptionExpression;
48+
import org.sonar.plugins.python.api.tree.Tree;
49+
import org.sonar.python.checks.Expressions;
50+
import org.sonar.python.tree.TreeUtils;
51+
52+
// https://jira.sonarsource.com/browse/SONARPY-668
53+
// https://jira.sonarsource.com/browse/RSPEC-5792
54+
@Rule(key = "S4502")
55+
public class CsrfDisabledCheck extends PythonSubscriptionCheck {
56+
57+
private static final String DISABLING_CSRF_MESSAGE = "Make sure disabling CSRF protection is safe here.";
58+
private static final String CSRFPROTECT_MISSING_MESSAGE = "Make sure not using CSRFProtect is safe here.";
59+
60+
@Override
61+
public void initialize(Context context) {
62+
context.registerSyntaxNodeConsumer(Tree.Kind.ASSIGNMENT_STMT, CsrfDisabledCheck::djangoMiddlewareArrayCheck);
63+
context.registerSyntaxNodeConsumer(Tree.Kind.DECORATOR, CsrfDisabledCheck::decoratorCsrfExemptCheck);
64+
context.registerSyntaxNodeConsumer(Tree.Kind.CALL_EXPR, CsrfDisabledCheck::functionCsrfExemptCheck);
65+
context.registerSyntaxNodeConsumer(Tree.Kind.ASSIGNMENT_STMT, CsrfDisabledCheck::flaskWtfCsrfEnabledFalseCheck);
66+
context.registerSyntaxNodeConsumer(Tree.Kind.CLASSDEF, CsrfDisabledCheck::metaCheck);
67+
context.registerSyntaxNodeConsumer(Tree.Kind.CALL_EXPR, CsrfDisabledCheck::formInstantiationCheck);
68+
context.registerSyntaxNodeConsumer(Tree.Kind.ASSIGNMENT_STMT, CsrfDisabledCheck::improperlyConfiguredFlaskApp);
69+
}
70+
71+
private static final String CSRF_VIEW_MIDDLEWARE = "django.middleware.csrf.CsrfViewMiddleware";
72+
73+
/** Checks that <code>django.middleware.csrf.CsrfViewMiddleware</code> is in <code>MIDDLEWARE</code> array. */
74+
private static void djangoMiddlewareArrayCheck(SubscriptionContext subscriptionContext) {
75+
if (!"settings.py".equals(subscriptionContext.pythonFile().fileName())) {
76+
return;
77+
}
78+
79+
AssignmentStatement asgn = (AssignmentStatement) subscriptionContext.syntaxNode();
80+
// Check that the left hand side is called `MIDDLEWARE` and that there is at least one string entry starting with
81+
// "django" in that array.
82+
boolean isLhsCalledMiddleware = isLhsCalled("MIDDLEWARE").test(asgn);
83+
boolean containsDjangoMiddleware = isListAnyMatch(isStringSatisfying(s -> s.startsWith("django"))).test(asgn.assignedValue());
84+
boolean isMiddlewareAssignment = isLhsCalledMiddleware && containsDjangoMiddleware;
85+
if (isMiddlewareAssignment) {
86+
boolean containsCsrfViewMiddleware = isListAnyMatch(isStringSatisfying(CSRF_VIEW_MIDDLEWARE::equals))
87+
.test(asgn.assignedValue());
88+
89+
if (!containsCsrfViewMiddleware) {
90+
subscriptionContext.addIssue(
91+
asgn.lastToken(),
92+
"Make sure not using CSRF protection (" + CSRF_VIEW_MIDDLEWARE + ") is safe here.");
93+
}
94+
}
95+
}
96+
97+
/** Checks that the left hand side of the assignment is a variable with name <code>lhsName</code>. */
98+
private static Predicate<AssignmentStatement> isLhsCalled(String lhsName) {
99+
return asgn -> asgn.lhsExpressions().stream()
100+
.flatMap(exprList -> exprList.expressions().stream())
101+
.anyMatch(expr -> expr.is(Tree.Kind.NAME) && lhsName.equals(((Name) expr).name()));
102+
}
103+
104+
/** Checks whether an expression is a string literal satisfying a predicate. */
105+
private static Predicate<Expression> isStringSatisfying(Predicate<String> pred) {
106+
return expr -> expr.is(Tree.Kind.STRING_LITERAL) && pred.test(((StringLiteral) expr).trimmedQuotesValue());
107+
}
108+
109+
/** Checks that an expression is a list literal with at least one entry satisfying the predicate. */
110+
private static Predicate<Expression> isListAnyMatch(Predicate<Expression> pred) {
111+
return expr -> Optional.ofNullable(expr)
112+
.filter(e -> e.is(Tree.Kind.LIST_LITERAL))
113+
.flatMap(lst -> ((ListLiteral) lst).elements().expressions().stream().filter(pred).findFirst())
114+
.isPresent();
115+
}
116+
117+
private static final Set<String> DANGEROUS_DECORATORS = new HashSet<>(Arrays.asList(
118+
"django.views.decorators.csrf.csrf_exempt",
119+
"flask_wtf.csrf.CSRFProtect.exempt"));
120+
121+
/** Raises issue whenever a decorator with something about "CSRF" and "exempt" in the combined name is found. */
122+
private static void decoratorCsrfExemptCheck(SubscriptionContext subscriptionContext) {
123+
Decorator decorator = (Decorator) subscriptionContext.syntaxNode();
124+
List<String> names = decorator.name().names().stream().map(Name::name).collect(Collectors.toList());
125+
// This is a temporary workaround until symbol resolution works for decorators.
126+
// Use the actual functions with FQNs from DANGEROUS_DECORATORS once that's fixed.
127+
// Related ticket: https://jira.sonarsource.com/browse/SONARPY-681
128+
boolean isDangerous = names.stream().anyMatch(s -> s.toLowerCase(Locale.US).contains("csrf")) &&
129+
names.stream().anyMatch(s -> s.toLowerCase(Locale.US).contains("exempt"));
130+
if (isDangerous) {
131+
subscriptionContext.addIssue(decorator.lastToken(), DISABLING_CSRF_MESSAGE);
132+
}
133+
}
134+
135+
/** Raises an issue whenever one of the CSRF-exemption decorators is used as an ordinary function. */
136+
private static void functionCsrfExemptCheck(SubscriptionContext subscriptionContext) {
137+
CallExpression callExpr = (CallExpression) subscriptionContext.syntaxNode();
138+
Optional.ofNullable(callExpr.calleeSymbol())
139+
.map(Symbol::fullyQualifiedName)
140+
.filter(DANGEROUS_DECORATORS::contains)
141+
.ifPresent(fqn -> subscriptionContext.addIssue(callExpr.callee().lastToken(), DISABLING_CSRF_MESSAGE));
142+
}
143+
144+
/** Checks that <code>'WTF_CSRF_ENABLED'</code> setting is not switched off. */
145+
private static void flaskWtfCsrfEnabledFalseCheck(SubscriptionContext subscriptionContext) {
146+
AssignmentStatement asgn = (AssignmentStatement) subscriptionContext.syntaxNode();
147+
// Checks that the left hand side is some kind of subscription of `something['WTF_CSRF_ENABLED']`
148+
// Does not check what `something` is - overtainting seems extremely unlikely in this case.
149+
boolean isWtfCsrfEnabledSubscription = asgn
150+
.lhsExpressions()
151+
.stream()
152+
.flatMap(exprList -> exprList.expressions().stream())
153+
.filter(expr -> expr.is(Tree.Kind.SUBSCRIPTION))
154+
.flatMap(s -> ((SubscriptionExpression) s).subscripts().expressions().stream())
155+
.anyMatch(isStringSatisfying(s -> "WTF_CSRF_ENABLED".equals(s) || "WTF_CSRF_CHECK_DEFAULT".equals(s)));
156+
if (isWtfCsrfEnabledSubscription && Expressions.isFalsy(asgn.assignedValue())) {
157+
subscriptionContext.addIssue(asgn.assignedValue(), DISABLING_CSRF_MESSAGE);
158+
}
159+
}
160+
161+
/**
162+
* Detects <code>class Meta</code> withing <code>FlaskForm</code>-subclasses,
163+
* with <code>csrf</code> set to <code>False</code>.
164+
*/
165+
private static void metaCheck(SubscriptionContext subscriptionContext) {
166+
ClassDef classDef = (ClassDef) subscriptionContext.syntaxNode();
167+
if (!"Meta".equals(classDef.name().name())) {
168+
return;
169+
}
170+
171+
boolean isWithinFlaskForm = Optional.ofNullable(TreeUtils.firstAncestorOfKind(classDef, Tree.Kind.CLASSDEF))
172+
.map(parentClassDef -> ((ClassDef) parentClassDef).name().symbol())
173+
.filter(s -> s.is(Symbol.Kind.CLASS))
174+
.map(ClassSymbol.class::cast)
175+
.filter(parentClassSymbol -> parentClassSymbol.canBeOrExtend("flask_wtf.FlaskForm"))
176+
.isPresent();
177+
if (!isWithinFlaskForm) {
178+
return;
179+
}
180+
181+
classDef.body().statements().forEach(stmt -> {
182+
if (stmt.is(Tree.Kind.ASSIGNMENT_STMT)) {
183+
AssignmentStatement asgn = (AssignmentStatement) stmt;
184+
if (isLhsCalled("csrf").test(asgn) && Expressions.isFalsy(asgn.assignedValue())) {
185+
subscriptionContext.addIssue(asgn.assignedValue(), DISABLING_CSRF_MESSAGE);
186+
}
187+
}
188+
});
189+
}
190+
191+
/** Checks that subclasses of <code>FlaskForm</code> are instantiated without bad CSRF settings. */
192+
private static void formInstantiationCheck(SubscriptionContext subscriptionContext) {
193+
CallExpression callExpr = (CallExpression) subscriptionContext.syntaxNode();
194+
boolean isFlaskFormInstantiation = Optional.ofNullable(callExpr.calleeSymbol())
195+
.filter(s -> s.is(Symbol.Kind.CLASS))
196+
.map(ClassSymbol.class::cast)
197+
.filter(c -> c.canBeOrExtend("flask_wtf.FlaskForm"))
198+
.isPresent();
199+
if (!isFlaskFormInstantiation) {
200+
return;
201+
}
202+
203+
callExpr.arguments().forEach(arg -> {
204+
if (arg instanceof RegularArgument) {
205+
RegularArgument regArg = (RegularArgument) arg;
206+
searchForProblemsInFormInitializationArguments(regArg)
207+
.ifPresent(badExpr -> subscriptionContext.addIssue(badExpr, DISABLING_CSRF_MESSAGE));
208+
}
209+
});
210+
}
211+
212+
/**
213+
* Attempts to find dangerous settings in a regular argument used in Flask form initialization.
214+
*/
215+
private static Optional<Expression> searchForProblemsInFormInitializationArguments(RegularArgument regArg) {
216+
String name = Optional.ofNullable(regArg.keywordArgument()).map(Name::name).orElse(null);
217+
if ("csrf_enabled".equals(name) && Expressions.isFalsy(regArg.expression())) {
218+
return Optional.of(regArg.expression());
219+
} else if ("meta".equals(name)) {
220+
return Optional.ofNullable(regArg.expression())
221+
.filter(s -> s.is(Tree.Kind.DICTIONARY_LITERAL))
222+
.map(DictionaryLiteral.class::cast)
223+
.flatMap(CsrfDisabledCheck::searchForBadCsrfSettingInDictionary);
224+
} else {
225+
return Optional.empty();
226+
}
227+
}
228+
229+
/** Looks for <code>'csrf': False</code> and similar settings in a dictionary. */
230+
private static Optional<Expression> searchForBadCsrfSettingInDictionary(DictionaryLiteral dict) {
231+
return dict.elements().stream()
232+
.filter(e -> e.is(Tree.Kind.KEY_VALUE_PAIR))
233+
.map(KeyValuePair.class::cast)
234+
.filter(kvp -> Optional.ofNullable(kvp.key())
235+
.filter(s -> s.is(Tree.Kind.STRING_LITERAL) && "csrf".equals(((StringLiteral) s).trimmedQuotesValue()))
236+
.isPresent())
237+
.findFirst()
238+
.filter(kvp -> Expressions.isFalsy(kvp.value()))
239+
.map(KeyValuePair::value);
240+
}
241+
242+
private static void improperlyConfiguredFlaskApp(SubscriptionContext subscriptionContext) {
243+
AssignmentStatement asgn = (AssignmentStatement) subscriptionContext.syntaxNode();
244+
if (isFlaskAppInstantiation(asgn.assignedValue())) {
245+
boolean isCsrfEnabledInThisFile = asgn.lhsExpressions().stream()
246+
.flatMap(exprList -> exprList.expressions().stream())
247+
.findFirst()
248+
.filter(s -> s.is(Tree.Kind.NAME))
249+
.flatMap(app -> Optional.of((Name) app)
250+
.map(Name::symbol)
251+
.map(Symbol::usages)
252+
.flatMap(usages -> usages.stream().filter(CsrfDisabledCheck::isWithinCsrfEnablingStatement).findFirst()))
253+
.isPresent();
254+
if (!isCsrfEnabledInThisFile) {
255+
subscriptionContext.addIssue(asgn.assignedValue(), CSRFPROTECT_MISSING_MESSAGE);
256+
}
257+
}
258+
}
259+
260+
/** Checks that an expression is some kind of <code>Flask(...)</code> constructor invocation. */
261+
private static boolean isFlaskAppInstantiation(Expression expr) {
262+
if (expr.is(Tree.Kind.CALL_EXPR)) {
263+
Symbol cs = ((CallExpression) expr).calleeSymbol();
264+
return cs != null && "flask.Flask".equals(cs.fullyQualifiedName());
265+
}
266+
return false;
267+
}
268+
269+
/** Detects usages like <code>CSRFProtect(a)</code>. */
270+
private static boolean isWithinCsrfEnablingStatement(Usage u) {
271+
Tree t = u.tree();
272+
return isWithinCall("flask_wtf.csrf.CSRFProtect", t) ||
273+
isWithinCall("flask_wtf.csrf.CSRFProtect.init_app", t);
274+
}
275+
276+
/** Checks that the surroundings of <code>t</code> look like <code>expectedCalleeFqn(someExpr(t))</code>. */
277+
private static boolean isWithinCall(String expectedCalleeFqn, Tree t) {
278+
Tree callExprTree = TreeUtils.firstAncestorOfKind(t, Tree.Kind.CALL_EXPR);
279+
if (callExprTree != null) {
280+
Symbol callExprSymb = ((CallExpression) callExprTree).calleeSymbol();
281+
return callExprSymb != null && expectedCalleeFqn.equals(callExprSymb.fullyQualifiedName());
282+
}
283+
return false;
284+
}
285+
}

0 commit comments

Comments
 (0)