Skip to content

Commit

Permalink
SONARPY-1468 Rule S6779: Flask secret keys should not be disclosed (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
Jeremi Do Dinh authored Nov 8, 2023
1 parent 49777dd commit bded873
Show file tree
Hide file tree
Showing 7 changed files with 475 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,7 @@ public static Iterable<Class> getChecks() {
FilePermissionsCheck.class,
FileHeaderCopyrightCheck.class,
FixmeCommentCheck.class,
FlaskHardCodedSecretCheck.class,
FloatingPointEqualityCheck.class,
FStringNestingLevelCheck.class,
FunctionComplexityCheck.class,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,190 @@
/*
* SonarQube Python Plugin
* Copyright (C) 2011-2023 SonarSource SA
* mailto:info AT sonarsource DOT com
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 3 of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public License
* along with this program; if not, write to the Free Software Foundation,
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/
package org.sonar.python.checks;

import java.util.List;
import java.util.Optional;
import java.util.Set;
import java.util.stream.Collectors;
import javax.annotation.Nullable;
import org.sonar.check.Rule;
import org.sonar.plugins.python.api.PythonSubscriptionCheck;
import org.sonar.plugins.python.api.SubscriptionContext;
import org.sonar.plugins.python.api.symbols.Symbol;
import org.sonar.plugins.python.api.tree.AssignmentStatement;
import org.sonar.plugins.python.api.tree.CallExpression;
import org.sonar.plugins.python.api.tree.DictionaryLiteral;
import org.sonar.plugins.python.api.tree.Expression;
import org.sonar.plugins.python.api.tree.ExpressionList;
import org.sonar.plugins.python.api.tree.KeyValuePair;
import org.sonar.plugins.python.api.tree.Name;
import org.sonar.plugins.python.api.tree.QualifiedExpression;
import org.sonar.plugins.python.api.tree.RegularArgument;
import org.sonar.plugins.python.api.tree.StringLiteral;
import org.sonar.plugins.python.api.tree.SubscriptionExpression;
import org.sonar.plugins.python.api.tree.Tree;
import org.sonar.python.tree.TreeUtils;


@Rule(key = "S6779")
public class FlaskHardCodedSecretCheck extends PythonSubscriptionCheck {
private static final String MESSAGE = "Don't disclose \"Flask\" secret keys.";
private static final String SECONDARY_MESSAGE = "Assignment to sensitive property.";
private static final String SECRET_KEY_KEYWORD = "SECRET_KEY";
private static final Set<String> FLASK_APP_CONFIG_QUALIFIER_FQNS = Set.of(
"flask.app.Flask.config",
"flask.globals.current_app.config"
);

private static final Set<String> FLASK_SECRET_KEY_FQNS = Set.of(
"flask.app.Flask.secret_key",
"flask.globals.current_app.secret_key"
);

@Override
public void initialize(Context context) {
context.registerSyntaxNodeConsumer(Tree.Kind.CALL_EXPR, FlaskHardCodedSecretCheck::verifyCallExpression);
context.registerSyntaxNodeConsumer(Tree.Kind.ASSIGNMENT_STMT, FlaskHardCodedSecretCheck::verifyAssignmentStatement);
}

private static void verifyCallExpression(SubscriptionContext ctx) {
CallExpression callExpression = (CallExpression) ctx.syntaxNode();
Optional.of(callExpression)
.map(CallExpression::callee)
.flatMap(TreeUtils.toOptionalInstanceOfMapper(QualifiedExpression.class))
.filter(qualiExpr -> "update".equals(qualiExpr.name().name()))
.map(QualifiedExpression::qualifier)
.flatMap(TreeUtils.toOptionalInstanceOfMapper(QualifiedExpression.class))
.map(QualifiedExpression::name)
.map(Name::symbol)
.map(Symbol::fullyQualifiedName)
.filter(FLASK_APP_CONFIG_QUALIFIER_FQNS::contains)
.ifPresent(fqn -> verifyUpdateCallArgument(ctx, callExpression));
}

private static void verifyUpdateCallArgument(SubscriptionContext ctx, CallExpression callExpression) {
Optional.of(callExpression.arguments())
.filter(arguments -> arguments.size() == 1)
.map(arguments -> arguments.get(0))
.flatMap(TreeUtils.toOptionalInstanceOfMapper(RegularArgument.class))
.map(RegularArgument::expression)
.map(FlaskHardCodedSecretCheck::getAssignedValue)
.filter(FlaskHardCodedSecretCheck::isIllegalDictArgument)
.ifPresent(expr -> ctx.addIssue(callExpression, MESSAGE));

}

private static Expression getAssignedValue(Expression expression) {
if (expression.is(Tree.Kind.NAME)) {
return Expressions.singleAssignedValue((Name) expression);
}
return expression;
}

private static boolean isIllegalDictArgument(Expression expression) {
if (expression.is(Tree.Kind.CALL_EXPR)) {
return isCallToDictConstructor((CallExpression) expression) && hasIllegalKeywordArgument((CallExpression) expression);
} else if (expression.is(Tree.Kind.DICTIONARY_LITERAL)) {
return hasIllegalKeyValuePair((DictionaryLiteral) expression);
}
return false;
}

private static boolean isCallToDictConstructor(CallExpression callExpression) {
return Optional.of(callExpression)
.map(CallExpression::callee)
.flatMap(TreeUtils.toOptionalInstanceOfMapper(Name.class))
.map(Name::symbol)
.map(Symbol::fullyQualifiedName)
.filter("dict"::equals)
.isPresent();
}

private static boolean hasIllegalKeyValuePair(DictionaryLiteral dictionaryLiteral) {
return dictionaryLiteral.elements().stream()
.filter(KeyValuePair.class::isInstance)
.map(KeyValuePair.class::cast)
.map(KeyValuePair::key)
.filter(StringLiteral.class::isInstance)
.map(StringLiteral.class::cast)
.map(StringLiteral::trimmedQuotesValue)
.anyMatch(SECRET_KEY_KEYWORD::equals);
}

private static boolean hasIllegalKeywordArgument(CallExpression callExpression) {
return Optional.ofNullable(TreeUtils.argumentByKeyword(SECRET_KEY_KEYWORD, callExpression.arguments()))
.map(RegularArgument::expression)
.filter(FlaskHardCodedSecretCheck::isStringLiteral)
.isPresent();
}

private static void verifyAssignmentStatement(SubscriptionContext ctx) {
AssignmentStatement assignmentStatementTree = (AssignmentStatement) ctx.syntaxNode();
List<Expression> expressionList = assignmentStatementTree.lhsExpressions().stream()
.map(ExpressionList::expressions)
.filter(list -> list.size() == 1)
.flatMap(List::stream)
.filter(FlaskHardCodedSecretCheck::isSensitiveProperty)
.filter(expression -> isStringLiteral(assignmentStatementTree.assignedValue()))
.collect(Collectors.toList());
if (!expressionList.isEmpty()) {
PreciseIssue issue = ctx.addIssue(assignmentStatementTree.assignedValue(), MESSAGE);
expressionList.forEach(expr -> issue.secondary(expr, SECONDARY_MESSAGE));
}
}

private static boolean isSensitiveProperty(Expression expression) {
if (expression.is(Tree.Kind.SUBSCRIPTION)) {
return Optional.of((SubscriptionExpression) expression)
.map(SubscriptionExpression::object)
.flatMap(TreeUtils.toOptionalInstanceOfMapper(QualifiedExpression.class))
.map(QualifiedExpression::symbol)
.map(Symbol::fullyQualifiedName)
.filter(FLASK_APP_CONFIG_QUALIFIER_FQNS::contains)
.map(fqn -> ((SubscriptionExpression) expression).subscripts())
.map(ExpressionList::expressions)
.filter(list -> list.size() == 1)
.map(list -> list.get(0))
.map(FlaskHardCodedSecretCheck::getAssignedValue)
.flatMap(TreeUtils.toOptionalInstanceOfMapper(StringLiteral.class))
.map(StringLiteral::trimmedQuotesValue)
.filter(SECRET_KEY_KEYWORD::equals)
.isPresent();
} else if (expression.is(Tree.Kind.QUALIFIED_EXPR)) {
return Optional.of((QualifiedExpression) expression)
.map(QualifiedExpression::symbol)
.map(Symbol::fullyQualifiedName)
.filter(FLASK_SECRET_KEY_FQNS::contains)
.isPresent();
}
return false;
}

private static boolean isStringLiteral(@Nullable Expression expr) {
if (expr == null) {
return false;
} else if (expr.is(Tree.Kind.NAME)) {
return isStringLiteral(Expressions.singleAssignedValue((Name) expr));
} else if (expr.is(Tree.Kind.STRING_LITERAL)) {
return true;
}
return false;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
<p>Secret leaks often occur when a sensitive piece of authentication data is stored with the source code of an application. Considering the source
code is intended to be deployed across multiple assets, including source code repositories or application hosting servers, the secrets might get
exposed to an unintended audience.</p>
<h2>Why is this an issue?</h2>
<p>In most cases, trust boundaries are violated when a secret is exposed in a source code repository or an uncontrolled deployment environment.
Unintended people who don’t need to know the secret might get access to it. They might then be able to use it to gain unwanted access to associated
services or resources.</p>
<p>The trust issue can be more or less severe depending on the people’s role and entitlement.</p>
<h3>What is the potential impact?</h3>
<p>If a Flask secret key leaks to an unintended audience, it can have serious security implications for the corresponding application. The secret key
is used to sign cookies and other sensitive data so that an attacker could potentially use it to perform malicious actions.</p>
<p>For example, an attacker could use the secret key to create their own cookies that appear to be legitimate, allowing them to bypass authentication
and gain access to sensitive data or functionality.</p>
<p>In the worst-case scenario, an attacker could be able to execute arbitrary code on the application and take over its hosting server.</p>
<h2>How to fix it</h2>
<p><strong>Revoke the secret</strong></p>
<p>Revoke any leaked secrets and remove them from the application source code.</p>
<p>Before revoking the secret, ensure that no other applications or processes are using it. Other usages of the secret will also be impacted when the
secret is revoked.</p>
<p>In Flask, changing the secret value is sufficient to invalidate any data that it protected.</p>
<p><strong>Use a secret vault</strong></p>
<p>A secret vault should be used to generate and store the new secret. This will ensure the secret’s security and prevent any further unexpected
disclosure.</p>
<p>Depending on the development platform and the leaked secret type, multiple solutions are currently available.</p>
<h3>Code examples</h3>
<h4>Noncompliant code example</h4>
<pre data-diff-id="1" data-diff-type="noncompliant">
from flask import Flask

app = Flask(__name__)
app.config['SECRET_KEY'] = "secret" # Noncompliant
</pre>
<h4>Compliant solution</h4>
<pre data-diff-id="1" data-diff-type="compliant">
from flask import Flask
import os

app = Flask(__name__)
app.config['SECRET_KEY'] = os.environ["SECRET_KEY"]
</pre>
<h2>Resources</h2>
<h3>Standards</h3>
<ul>
<li> MITRE - <a href="https://cwe.mitre.org/data/definitions/798">CWE-798 - Use of Hard-coded Credentials</a> </li>
<li> MITRE - <a href="https://cwe.mitre.org/data/definitions/259">CWE-259 - Use of Hard-coded Password</a> </li>
</ul>
<h3>Documentation</h3>
<ul>
<li> Flask documentation - <a href="https://flask.palletsprojects.com/en/2.3.x/config/#SECRET_KEY">Config - SECRET_KEY</a> </li>
</ul>

Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
{
"title": "Flask secret keys should not be disclosed",
"type": "VULNERABILITY",
"code": {
"impacts": {
"SECURITY": "HIGH"
},
"attribute": "TRUSTWORTHY"
},
"status": "ready",
"remediation": {
"func": "Constant\/Issue",
"constantCost": "30min"
},
"tags": [
"cwe"
],
"defaultSeverity": "Blocker",
"ruleSpecification": "RSPEC-6779",
"sqKey": "S6779",
"scope": "All",
"securityStandards": {
"CWE": [
798,
259
],
"OWASP": [
"A3"
],
"CERT": [
"MSC03-J."
],
"OWASP Top 10 2021": [
"A7"
],
"PCI DSS 3.2": [
"6.5.10"
],
"PCI DSS 4.0": [
"6.2.4"
],
"ASVS 4.0": [
"2.10.4",
"3.5.2",
"6.4.1"
]
},
"quickfix": "unknown"
}
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,7 @@
"S6735",
"S6741",
"S6742",
"S6779",
"S6792",
"S6794",
"S6796",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/*
* SonarQube Python Plugin
* Copyright (C) 2011-2023 SonarSource SA
* mailto:info AT sonarsource DOT com
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 3 of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public License
* along with this program; if not, write to the Free Software Foundation,
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/
package org.sonar.python.checks;

import org.junit.jupiter.api.Test;
import org.sonar.python.checks.utils.PythonCheckVerifier;

class FlaskHardCodedSecretCheckTest {

@Test
void test() {
PythonCheckVerifier.verify("src/test/resources/checks/flaskHardCodedSecret.py", new FlaskHardCodedSecretCheck());
}
}
Loading

0 comments on commit bded873

Please sign in to comment.