Skip to content

Commit

Permalink
SONARPY-1547 Rule S3330: Add support for FastAPI (#1652)
Browse files Browse the repository at this point in the history
  • Loading branch information
@maksim-grebeniuk-sonarsource
maksim-grebeniuk-sonarsource authored Nov 20, 2023
1 parent eaa9f28 commit 7d5c095
Show file tree
Hide file tree
Showing 4 changed files with 153 additions and 71 deletions.
45 changes: 45 additions & 0 deletions python-checks/src/main/java/org/sonar/python/checks/hotspots/AbstractCookieFlagCheck.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,17 @@
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.Kind;
import org.sonar.python.checks.Expressions;
import org.sonar.python.tree.TreeUtils;

import static org.sonar.python.checks.Expressions.isFalsy;
Expand Down Expand Up @@ -168,6 +172,47 @@ private static Expression getObject(Expression object) {
return object;
}

protected boolean isInvalidHeaderArgument(@Nullable RegularArgument argument) {
return Optional.ofNullable(argument)
.map(RegularArgument::expression)
.map(this::isDictWithSensitiveEntry)
.orElse(false);
}

private boolean isDictWithSensitiveEntry(Expression expression) {
return TreeUtils.toOptionalInstanceOf(Name.class, expression)
.map(Expressions::singleAssignedNonNameValue)
.map(this::isDictWithSensitiveEntry)
.or(() -> TreeUtils.toOptionalInstanceOf(DictionaryLiteral.class, expression)
.map(this::hasInvalidEntry)
).orElse(false);
}

private boolean hasInvalidEntry(DictionaryLiteral dictionaryLiteral) {
return dictionaryLiteral.elements().stream()
.filter(KeyValuePair.class::isInstance)
.map(KeyValuePair.class::cast)
.filter(keyValuePair -> isSensitiveKey(keyValuePair.key()))
.map(KeyValuePair::value)
.anyMatch(this::invalidValue);
}

private static boolean isSensitiveKey(Expression key) {
return TreeUtils.toOptionalInstanceOf(StringLiteral.class, key)
.map(StringLiteral::trimmedQuotesValue)
.filter("set-cookie"::equalsIgnoreCase)
.isPresent();
}

private boolean invalidValue(Expression value) {
return TreeUtils.toOptionalInstanceOf(StringLiteral.class, value)
.map(StringLiteral::trimmedQuotesValue)
.filter(Predicate.not(val -> val.matches(headerValueRegex())))
.isPresent();
}

protected abstract String headerValueRegex();

abstract String flagName();

abstract String message();
Expand Down
54 changes: 47 additions & 7 deletions python-checks/src/main/java/org/sonar/python/checks/hotspots/HttpOnlyCookieCheck.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,49 @@
public class HttpOnlyCookieCheck extends AbstractCookieFlagCheck {

public static final String HTTPONLY_ARGUMENT_NAME = "httponly";
public static final String HEADERS_ARGUMENT_NAME = "headers";
public static final String SET_COOKIE_METHOD_NAME = "set_cookie";

private final MethodArgumentsToCheckRegistry methodArgumentsToCheckRegistry = new MethodArgumentsToCheckRegistry(
new MethodArgumentsToCheck("django.http.response.HttpResponseBase", SET_COOKIE_METHOD_NAME, HTTPONLY_ARGUMENT_NAME, 7),
new MethodArgumentsToCheck("django.http.response.HttpResponseBase", "set_signed_cookie", HTTPONLY_ARGUMENT_NAME, 8),
new MethodArgumentsToCheck("flask.wrappers.Response", SET_COOKIE_METHOD_NAME, HTTPONLY_ARGUMENT_NAME, 7),
new MethodArgumentsToCheck("werkzeug.wrappers.BaseResponse", SET_COOKIE_METHOD_NAME, HTTPONLY_ARGUMENT_NAME, 7),
new MethodArgumentsToCheck("werkzeug.sansio.response.Response", SET_COOKIE_METHOD_NAME, HTTPONLY_ARGUMENT_NAME, 7),
// check for set_cookie method httponly param
new MethodArgumentsToCheck("fastapi.Response", SET_COOKIE_METHOD_NAME, HTTPONLY_ARGUMENT_NAME, -1),
new MethodArgumentsToCheck("fastapi.responses.Response", SET_COOKIE_METHOD_NAME, HTTPONLY_ARGUMENT_NAME, -1),
new MethodArgumentsToCheck("starlette.responses.Response", SET_COOKIE_METHOD_NAME, HTTPONLY_ARGUMENT_NAME, -1),
new MethodArgumentsToCheck("fastapi.responses.HTMLResponse", SET_COOKIE_METHOD_NAME, HTTPONLY_ARGUMENT_NAME, -1),
new MethodArgumentsToCheck("starlette.responses.HTMLResponse", SET_COOKIE_METHOD_NAME, HTTPONLY_ARGUMENT_NAME, -1),
new MethodArgumentsToCheck("fastapi.responses.JSONResponse", SET_COOKIE_METHOD_NAME, HTTPONLY_ARGUMENT_NAME, -1),
new MethodArgumentsToCheck("starlette.responses.JSONResponse", SET_COOKIE_METHOD_NAME, HTTPONLY_ARGUMENT_NAME, -1),
new MethodArgumentsToCheck("fastapi.responses.ORJSONResponse", SET_COOKIE_METHOD_NAME, HTTPONLY_ARGUMENT_NAME, -1),
new MethodArgumentsToCheck("fastapi.responses.PlainTextResponse", SET_COOKIE_METHOD_NAME, HTTPONLY_ARGUMENT_NAME, -1),
new MethodArgumentsToCheck("starlette.responses.PlainTextResponse", SET_COOKIE_METHOD_NAME, HTTPONLY_ARGUMENT_NAME, -1),
new MethodArgumentsToCheck("fastapi.responses.StreamingResponse", SET_COOKIE_METHOD_NAME, HTTPONLY_ARGUMENT_NAME, -1),
new MethodArgumentsToCheck("starlette.responses.StreamingResponse", SET_COOKIE_METHOD_NAME, HTTPONLY_ARGUMENT_NAME, -1),
new MethodArgumentsToCheck("fastapi.responses.UJSONResponse", SET_COOKIE_METHOD_NAME, HTTPONLY_ARGUMENT_NAME, -1),
new MethodArgumentsToCheck("fastapi.responses.FileResponse", SET_COOKIE_METHOD_NAME, HTTPONLY_ARGUMENT_NAME, -1),
new MethodArgumentsToCheck("starlette.responses.FileResponse", SET_COOKIE_METHOD_NAME, HTTPONLY_ARGUMENT_NAME, -1),
// check for set-cookie header constructor param
new MethodArgumentsToCheck("fastapi.Response", HEADERS_ARGUMENT_NAME, -1, this::isInvalidHeaderArgument),
new MethodArgumentsToCheck("fastapi.responses.Response", HEADERS_ARGUMENT_NAME, -1, this::isInvalidHeaderArgument),
new MethodArgumentsToCheck("starlette.responses.Response", HEADERS_ARGUMENT_NAME, -1, this::isInvalidHeaderArgument),
new MethodArgumentsToCheck("fastapi.responses.HTMLResponse", HEADERS_ARGUMENT_NAME, -1, this::isInvalidHeaderArgument),
new MethodArgumentsToCheck("starlette.responses.HTMLResponse", HEADERS_ARGUMENT_NAME, -1, this::isInvalidHeaderArgument),
new MethodArgumentsToCheck("fastapi.responses.JSONResponse", HEADERS_ARGUMENT_NAME, -1, this::isInvalidHeaderArgument),
new MethodArgumentsToCheck("starlette.responses.JSONResponse", HEADERS_ARGUMENT_NAME, -1, this::isInvalidHeaderArgument),
new MethodArgumentsToCheck("fastapi.responses.ORJSONResponse", HEADERS_ARGUMENT_NAME, -1, this::isInvalidHeaderArgument),
new MethodArgumentsToCheck("fastapi.responses.PlainTextResponse", HEADERS_ARGUMENT_NAME, -1, this::isInvalidHeaderArgument),
new MethodArgumentsToCheck("starlette.responses.PlainTextResponse", HEADERS_ARGUMENT_NAME, -1, this::isInvalidHeaderArgument),
new MethodArgumentsToCheck("fastapi.responses.StreamingResponse", HEADERS_ARGUMENT_NAME, -1, this::isInvalidHeaderArgument),
new MethodArgumentsToCheck("starlette.responses.StreamingResponse", HEADERS_ARGUMENT_NAME, -1, this::isInvalidHeaderArgument),
new MethodArgumentsToCheck("fastapi.responses.UJSONResponse", HEADERS_ARGUMENT_NAME, -1, this::isInvalidHeaderArgument),
new MethodArgumentsToCheck("fastapi.responses.FileResponse", HEADERS_ARGUMENT_NAME, -1, this::isInvalidHeaderArgument),
new MethodArgumentsToCheck("starlette.responses.FileResponse", HEADERS_ARGUMENT_NAME, -1, this::isInvalidHeaderArgument)
);

@Override
String flagName() {
return HTTPONLY_ARGUMENT_NAME;
Expand All @@ -54,13 +95,7 @@ String message() {

@Override
MethodArgumentsToCheckRegistry methodArgumentsToCheckRegistry() {
return new MethodArgumentsToCheckRegistry(
new MethodArgumentsToCheck("django.http.response.HttpResponseBase", SET_COOKIE_METHOD_NAME, HTTPONLY_ARGUMENT_NAME, 7),
new MethodArgumentsToCheck("django.http.response.HttpResponseBase", "set_signed_cookie" , HTTPONLY_ARGUMENT_NAME, 8),
new MethodArgumentsToCheck("flask.wrappers.Response", SET_COOKIE_METHOD_NAME, HTTPONLY_ARGUMENT_NAME, 7),
new MethodArgumentsToCheck("werkzeug.wrappers.BaseResponse", SET_COOKIE_METHOD_NAME, HTTPONLY_ARGUMENT_NAME, 7),
new MethodArgumentsToCheck("werkzeug.sansio.response.Response", SET_COOKIE_METHOD_NAME, HTTPONLY_ARGUMENT_NAME, 7)
);
return methodArgumentsToCheckRegistry;
}


Expand All @@ -81,6 +116,11 @@ public void initialize(Context context) {
);
}

@Override
protected String headerValueRegex() {
return ".*;\\s?HttpOnly.*";
}

private void subscriptionSessionCookieHttponlyCheck(SubscriptionContext ctx) {
AssignmentStatement assignmentStatement = (AssignmentStatement) ctx.syntaxNode();
boolean isSubscriptionToSessionCookieHttponly = assignmentStatement
Expand Down
84 changes: 20 additions & 64 deletions python-checks/src/main/java/org/sonar/python/checks/hotspots/SecureCookieCheck.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,26 +19,16 @@
*/
package org.sonar.python.checks.hotspots;

import java.util.Optional;
import java.util.function.Predicate;
import javax.annotation.Nullable;
import org.sonar.check.Rule;
import org.sonar.plugins.python.api.tree.DictionaryLiteral;
import org.sonar.plugins.python.api.tree.Expression;
import org.sonar.plugins.python.api.tree.KeyValuePair;
import org.sonar.plugins.python.api.tree.Name;
import org.sonar.plugins.python.api.tree.RegularArgument;
import org.sonar.plugins.python.api.tree.StringLiteral;
import org.sonar.python.checks.Expressions;
import org.sonar.python.tree.TreeUtils;

@Rule(key = "S2092")
public class SecureCookieCheck extends AbstractCookieFlagCheck {

public static final String SET_COOKIE_METHOD_NAME = "set_cookie";
public static final String SECURE_ARGUMENT_NAME = "secure";
public static final String HEADERS_ARGUMENT_NAME = "headers";
private static final MethodArgumentsToCheckRegistry METHOD_ARGUMENTS_TO_CHECK_REGISTRY = new MethodArgumentsToCheckRegistry(

private final MethodArgumentsToCheckRegistry methodArgumentsToCheckRegistry = new MethodArgumentsToCheckRegistry(
// Check for falsy secure argument
new MethodArgumentsToCheck("django.http.response.HttpResponseBase", SET_COOKIE_METHOD_NAME, SECURE_ARGUMENT_NAME, 6),
new MethodArgumentsToCheck("flask.wrappers.Response", SET_COOKIE_METHOD_NAME, SECURE_ARGUMENT_NAME, 6),
Expand All @@ -61,20 +51,20 @@ public class SecureCookieCheck extends AbstractCookieFlagCheck {
new MethodArgumentsToCheck("fastapi.responses.FileResponse", SET_COOKIE_METHOD_NAME, SECURE_ARGUMENT_NAME, -1),
new MethodArgumentsToCheck("starlette.responses.FileResponse", SET_COOKIE_METHOD_NAME, SECURE_ARGUMENT_NAME, -1),
// check for insecure set-cookie header
new MethodArgumentsToCheck("fastapi.responses.Response", HEADERS_ARGUMENT_NAME, -1, SecureCookieCheck::isInvalidHeaderArgument),
new MethodArgumentsToCheck("starlette.responses.Response", HEADERS_ARGUMENT_NAME, -1, SecureCookieCheck::isInvalidHeaderArgument),
new MethodArgumentsToCheck("fastapi.responses.HTMLResponse", HEADERS_ARGUMENT_NAME, -1, SecureCookieCheck::isInvalidHeaderArgument),
new MethodArgumentsToCheck("starlette.responses.HTMLResponse", HEADERS_ARGUMENT_NAME, -1, SecureCookieCheck::isInvalidHeaderArgument),
new MethodArgumentsToCheck("fastapi.responses.JSONResponse", HEADERS_ARGUMENT_NAME, -1, SecureCookieCheck::isInvalidHeaderArgument),
new MethodArgumentsToCheck("starlette.responses.JSONResponse", HEADERS_ARGUMENT_NAME, -1, SecureCookieCheck::isInvalidHeaderArgument),
new MethodArgumentsToCheck("fastapi.responses.ORJSONResponse", HEADERS_ARGUMENT_NAME, -1, SecureCookieCheck::isInvalidHeaderArgument),
new MethodArgumentsToCheck("fastapi.responses.PlainTextResponse", HEADERS_ARGUMENT_NAME, -1, SecureCookieCheck::isInvalidHeaderArgument),
new MethodArgumentsToCheck("starlette.responses.PlainTextResponse", HEADERS_ARGUMENT_NAME, -1, SecureCookieCheck::isInvalidHeaderArgument),
new MethodArgumentsToCheck("fastapi.responses.StreamingResponse", HEADERS_ARGUMENT_NAME, -1, SecureCookieCheck::isInvalidHeaderArgument),
new MethodArgumentsToCheck("starlette.responses.StreamingResponse", HEADERS_ARGUMENT_NAME, -1, SecureCookieCheck::isInvalidHeaderArgument),
new MethodArgumentsToCheck("fastapi.responses.UJSONResponse", HEADERS_ARGUMENT_NAME, -1, SecureCookieCheck::isInvalidHeaderArgument),
new MethodArgumentsToCheck("fastapi.responses.FileResponse", HEADERS_ARGUMENT_NAME, -1, SecureCookieCheck::isInvalidHeaderArgument),
new MethodArgumentsToCheck("starlette.responses.FileResponse", HEADERS_ARGUMENT_NAME, -1, SecureCookieCheck::isInvalidHeaderArgument)
new MethodArgumentsToCheck("fastapi.responses.Response", HEADERS_ARGUMENT_NAME, -1, this::isInvalidHeaderArgument),
new MethodArgumentsToCheck("starlette.responses.Response", HEADERS_ARGUMENT_NAME, -1, this::isInvalidHeaderArgument),
new MethodArgumentsToCheck("fastapi.responses.HTMLResponse", HEADERS_ARGUMENT_NAME, -1, this::isInvalidHeaderArgument),
new MethodArgumentsToCheck("starlette.responses.HTMLResponse", HEADERS_ARGUMENT_NAME, -1, this::isInvalidHeaderArgument),
new MethodArgumentsToCheck("fastapi.responses.JSONResponse", HEADERS_ARGUMENT_NAME, -1, this::isInvalidHeaderArgument),
new MethodArgumentsToCheck("starlette.responses.JSONResponse", HEADERS_ARGUMENT_NAME, -1, this::isInvalidHeaderArgument),
new MethodArgumentsToCheck("fastapi.responses.ORJSONResponse", HEADERS_ARGUMENT_NAME, -1, this::isInvalidHeaderArgument),
new MethodArgumentsToCheck("fastapi.responses.PlainTextResponse", HEADERS_ARGUMENT_NAME, -1, this::isInvalidHeaderArgument),
new MethodArgumentsToCheck("starlette.responses.PlainTextResponse", HEADERS_ARGUMENT_NAME, -1, this::isInvalidHeaderArgument),
new MethodArgumentsToCheck("fastapi.responses.StreamingResponse", HEADERS_ARGUMENT_NAME, -1, this::isInvalidHeaderArgument),
new MethodArgumentsToCheck("starlette.responses.StreamingResponse", HEADERS_ARGUMENT_NAME, -1, this::isInvalidHeaderArgument),
new MethodArgumentsToCheck("fastapi.responses.UJSONResponse", HEADERS_ARGUMENT_NAME, -1, this::isInvalidHeaderArgument),
new MethodArgumentsToCheck("fastapi.responses.FileResponse", HEADERS_ARGUMENT_NAME, -1, this::isInvalidHeaderArgument),
new MethodArgumentsToCheck("starlette.responses.FileResponse", HEADERS_ARGUMENT_NAME, -1, this::isInvalidHeaderArgument)
);

@Override
Expand All @@ -89,45 +79,11 @@ String message() {

@Override
MethodArgumentsToCheckRegistry methodArgumentsToCheckRegistry() {
return METHOD_ARGUMENTS_TO_CHECK_REGISTRY;
}

private static boolean isInvalidHeaderArgument(@Nullable RegularArgument argument) {
return Optional.ofNullable(argument)
.map(RegularArgument::expression)
.map(SecureCookieCheck::isDictWithSensitiveEntry)
.orElse(false);
}

private static boolean isDictWithSensitiveEntry(Expression expression) {
return TreeUtils.toOptionalInstanceOf(Name.class, expression)
.map(Expressions::singleAssignedNonNameValue)
.map(SecureCookieCheck::isDictWithSensitiveEntry)
.or(() -> TreeUtils.toOptionalInstanceOf(DictionaryLiteral.class, expression)
.map(SecureCookieCheck::hasTrueVerifySignatureEntry)
).orElse(false);
return methodArgumentsToCheckRegistry;
}

private static boolean hasTrueVerifySignatureEntry(DictionaryLiteral dictionaryLiteral) {
return dictionaryLiteral.elements().stream()
.filter(KeyValuePair.class::isInstance)
.map(KeyValuePair.class::cast)
.filter(keyValuePair -> isSensitiveKey(keyValuePair.key()))
.map(KeyValuePair::value)
.anyMatch(SecureCookieCheck::invalidSetCookieHeaderValue);
}

private static boolean isSensitiveKey(Expression key) {
return TreeUtils.toOptionalInstanceOf(StringLiteral.class, key)
.map(StringLiteral::trimmedQuotesValue)
.filter("set-cookie"::equalsIgnoreCase)
.isPresent();
}

private static boolean invalidSetCookieHeaderValue(Expression value) {
return TreeUtils.toOptionalInstanceOf(StringLiteral.class, value)
.map(StringLiteral::trimmedQuotesValue)
.filter(Predicate.not(val -> val.matches(".*;\\s?Secure")))
.isPresent();
@Override
protected String headerValueRegex() {
return ".*;\\s?Secure.*";
}
}
41 changes: 41 additions & 0 deletions python-checks/src/test/resources/checks/hotspots/httpOnlyCookie.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,3 +118,44 @@ def flask_SessionCookieHttpOnlyFalse():
SECRET_KEY = "woopie",
SESSION_COOKIE_HTTPONLY = True # Ok
))

from fastapi import FastAPI, Response
from fastapi.responses import HTMLResponse, PlainTextResponse

app = FastAPI()


@app.get("/")
async def root():
return {"message": "Hello World"}


@app.get("/test1")
async def test1(response: Response):
response.set_cookie("key", "value", httponly=False) # Noncompliant
return {"message": "test"}


@app.get("/test2")
async def test2(response: Response):
response.set_cookie("key", "value") # Noncompliant
return {"message": "test"}


@app.get("/test3")
async def test3(response: Response):
response.set_cookie("key", "value", httponly=True) # OK
return {"message": "test"}


@app.get("/test4")
async def test4():
response = HTMLResponse("<html>test</html>")
response.set_cookie("key", "value") # Noncompliant
return response


@app.get("/test5")
async def test5():
response = PlainTextResponse("test", headers={"set-cookie": "key=value"}) # Noncompliant
return response

0 comments on commit 7d5c095

Please sign in to comment.