diff --git a/python-checks/src/main/java/org/sonar/python/checks/hotspots/AbstractCookieFlagCheck.java b/python-checks/src/main/java/org/sonar/python/checks/hotspots/AbstractCookieFlagCheck.java index 9fc59cc937..0730b4b71b 100644 --- a/python-checks/src/main/java/org/sonar/python/checks/hotspots/AbstractCookieFlagCheck.java +++ b/python-checks/src/main/java/org/sonar/python/checks/hotspots/AbstractCookieFlagCheck.java @@ -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; @@ -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(); diff --git a/python-checks/src/main/java/org/sonar/python/checks/hotspots/HttpOnlyCookieCheck.java b/python-checks/src/main/java/org/sonar/python/checks/hotspots/HttpOnlyCookieCheck.java index a1ac5db5f5..584412ec6a 100644 --- a/python-checks/src/main/java/org/sonar/python/checks/hotspots/HttpOnlyCookieCheck.java +++ b/python-checks/src/main/java/org/sonar/python/checks/hotspots/HttpOnlyCookieCheck.java @@ -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; @@ -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; } @@ -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 diff --git a/python-checks/src/main/java/org/sonar/python/checks/hotspots/SecureCookieCheck.java b/python-checks/src/main/java/org/sonar/python/checks/hotspots/SecureCookieCheck.java index 6cc47bae75..f72f01b937 100644 --- a/python-checks/src/main/java/org/sonar/python/checks/hotspots/SecureCookieCheck.java +++ b/python-checks/src/main/java/org/sonar/python/checks/hotspots/SecureCookieCheck.java @@ -19,18 +19,7 @@ */ 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 { @@ -38,7 +27,8 @@ 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), @@ -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 @@ -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.*"; } } diff --git a/python-checks/src/test/resources/checks/hotspots/httpOnlyCookie.py b/python-checks/src/test/resources/checks/hotspots/httpOnlyCookie.py index 2d2ec941f7..731794b73c 100644 --- a/python-checks/src/test/resources/checks/hotspots/httpOnlyCookie.py +++ b/python-checks/src/test/resources/checks/hotspots/httpOnlyCookie.py @@ -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("test") + 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