diff --git a/integration_tests/sonar/test_sonar_secure_cookie.py b/integration_tests/sonar/test_sonar_secure_cookie.py new file mode 100644 index 00000000..409592ae --- /dev/null +++ b/integration_tests/sonar/test_sonar_secure_cookie.py @@ -0,0 +1,19 @@ +from codemodder.codemods.test import SonarIntegrationTest +from core_codemods.sonar.sonar_secure_cookie import ( + SonarSecureCookie, + SonarSecureCookieTransformer, +) + + +class TestSonarSecureCookie(SonarIntegrationTest): + codemod = SonarSecureCookie + code_path = "tests/samples/secure_cookie.py" + replacement_lines = [ + ( + 8, + """ resp.set_cookie('custom_cookie', 'value', secure=True, httponly=True, samesite='Lax')\n""", + ), + ] + expected_diff = "--- \n+++ \n@@ -5,5 +5,5 @@\n @app.route('/')\n def index():\n resp = make_response('Custom Cookie Set')\n- resp.set_cookie('custom_cookie', 'value')\n+ resp.set_cookie('custom_cookie', 'value', secure=True, httponly=True, samesite='Lax')\n return resp\n" + expected_line_change = "8" + change_description = SonarSecureCookieTransformer.change_description diff --git a/integration_tests/test_secure_flask_cookie.py b/integration_tests/test_secure_flask_cookie.py index b49c155d..d9752f84 100644 --- a/integration_tests/test_secure_flask_cookie.py +++ b/integration_tests/test_secure_flask_cookie.py @@ -1,5 +1,5 @@ from codemodder.codemods.test import BaseIntegrationTest -from core_codemods.secure_flask_cookie import SecureFlaskCookie +from core_codemods.secure_flask_cookie import SecureCookieTransformer, SecureFlaskCookie class TestSecureFlaskCookie(BaseIntegrationTest): @@ -23,4 +23,4 @@ def index(): ] expected_diff = "--- \n+++ \n@@ -5,5 +5,5 @@\n @app.route('/')\n def index():\n resp = make_response('Custom Cookie Set')\n- resp.set_cookie('custom_cookie', 'value')\n+ resp.set_cookie('custom_cookie', 'value', secure=True, httponly=True, samesite='Lax')\n return resp\n" expected_line_change = "8" - change_description = SecureFlaskCookie.change_description + change_description = SecureCookieTransformer.change_description diff --git a/src/codemodder/codemods/test/integration_utils.py b/src/codemodder/codemods/test/integration_utils.py index 262dc871..ae191227 100644 --- a/src/codemodder/codemods/test/integration_utils.py +++ b/src/codemodder/codemods/test/integration_utils.py @@ -124,8 +124,12 @@ def _assert_results_fields(self, results, output_path): # TODO: if/when we add description for each url for reference in result["references"][ - # Last reference for Sonar has a different description - : (-1 if self.sonar_issues_json or self.sonar_hotspots_json else None) + # Last references for Sonar has a different description + : ( + -len(self.codemod.requested_rules) + if self.sonar_issues_json or self.sonar_hotspots_json + else None + ) ]: assert reference["url"] == reference["description"] @@ -288,21 +292,25 @@ def check_sonar_issues(cls): (cls.sonar_issues_json, cls.sonar_hotspots_json) ) - assert ( - cls.codemod.requested_rules[-1] in sonar_results + assert any( + [x in sonar_results for x in cls.codemod.requested_rules] ), f"Make sure to add a sonar issue/hotspot for {cls.codemod.rule_id} in {cls.sonar_issues_json} or {cls.sonar_hotspots_json}" results_for_codemod = sonar_results[cls.codemod.requested_rules[-1]] file_path = pathlib.Path(cls.code_filename) assert ( file_path in results_for_codemod - ), f"Make sure to add a sonar issue/hotspot for file `{cls.code_filename}` under rule `{cls.codemod.rule_id}`in {cls.sonar_issues_json} or {cls.sonar_hotspots_json}" + ), f"Make sure to add a sonar issue/hotspot for file `{cls.code_filename}` under one of the rules `{cls.codemod.requested_rules}`in {cls.sonar_issues_json} or {cls.sonar_hotspots_json}" def _assert_sonar_fields(self, result): assert self.codemod_instance._metadata.tool is not None - assert ( - result["references"][-1]["description"] - == self.codemod_instance._metadata.tool.rules[0].name - ) + rules = self.codemod_instance._metadata.tool.rules + for i in range(len(rules)): + assert ( + result["references"][len(result["references"]) - len(rules) + i][ + "description" + ] + == self.codemod_instance._metadata.tool.rules[i].name + ) assert result["detectionTool"]["name"] == "Sonar" diff --git a/src/codemodder/scripts/generate_docs.py b/src/codemodder/scripts/generate_docs.py index 42e03503..63a0095c 100644 --- a/src/codemodder/scripts/generate_docs.py +++ b/src/codemodder/scripts/generate_docs.py @@ -328,6 +328,12 @@ class DocMetadata: need_sarif="Yes (Sonar)", ) for name in SONAR_CODEMOD_NAMES +} | { + "secure-cookie": DocMetadata( + importance="Medium", + guidance_explained="Our change provides the most secure way to create cookies in Flask. However, it's possible you have configured your Flask application configurations to use secure cookies. In these cases, using the default parameters for `set_cookie` is safe.", + need_sarif="Yes (Sonar)", + ), } SEMGREP_CODEMOD_NAMES = [ diff --git a/src/core_codemods/__init__.py b/src/core_codemods/__init__.py index fdc71435..0ae2c15c 100644 --- a/src/core_codemods/__init__.py +++ b/src/core_codemods/__init__.py @@ -89,6 +89,7 @@ SonarRemoveAssertionInPytestRaises, ) from .sonar.sonar_sandbox_process_creation import SonarSandboxProcessCreation +from .sonar.sonar_secure_cookie import SonarSecureCookie from .sonar.sonar_secure_random import SonarSecureRandom from .sonar.sonar_sql_parameterization import SonarSQLParameterization from .sonar.sonar_tempfile_mktemp import SonarTempfileMktemp @@ -204,6 +205,7 @@ SonarInvertedBooleanCheck, SonarTimezoneAwareDatetime, SonarSandboxProcessCreation, + SonarSecureCookie, ], ) diff --git a/src/core_codemods/secure_flask_cookie.py b/src/core_codemods/secure_flask_cookie.py index 41f5445f..8e72d45a 100644 --- a/src/core_codemods/secure_flask_cookie.py +++ b/src/core_codemods/secure_flask_cookie.py @@ -1,9 +1,25 @@ -from core_codemods.api import Metadata, Reference, ReviewGuidance, SimpleCodemod +from codemodder.codemods.libcst_transformer import ( + LibcstResultTransformer, + LibcstTransformerPipeline, +) +from codemodder.codemods.semgrep import SemgrepRuleDetector +from core_codemods.api import Metadata, Reference, ReviewGuidance +from core_codemods.api.core_codemod import CoreCodemod from core_codemods.secure_cookie_mixin import SecureCookieMixin -class SecureFlaskCookie(SimpleCodemod, SecureCookieMixin): - metadata = Metadata( +class SecureCookieTransformer(LibcstResultTransformer, SecureCookieMixin): + change_description = "Flask response `set_cookie` call should be called with `secure=True`, `httponly=True`, and `samesite='Lax'`." + + def on_result_found(self, original_node, updated_node): + new_args = self.replace_args( + original_node, self._choose_new_args(original_node) + ) + return self.update_arg_target(updated_node, new_args) + + +SecureFlaskCookie = CoreCodemod( + metadata=Metadata( name="secure-flask-cookie", summary="Use Safe Parameters in `flask` Response `set_cookie` Call", review_guidance=ReviewGuidance.MERGE_AFTER_CURSORY_REVIEW, @@ -14,11 +30,14 @@ class SecureFlaskCookie(SimpleCodemod, SecureCookieMixin): Reference( url="https://owasp.org/www-community/controls/SecureCookieAttribute" ), + Reference(url="https://cwe.mitre.org/data/definitions/1004"), + Reference(url="https://cwe.mitre.org/data/definitions/311"), + Reference(url="https://cwe.mitre.org/data/definitions/315"), Reference(url="https://cwe.mitre.org/data/definitions/614"), ], - ) - change_description = "Flask response `set_cookie` call should be called with `secure=True`, `httponly=True`, and `samesite='Lax'`." - detector_pattern = """ + ), + detector=SemgrepRuleDetector( + """ rules: - id: secure-flask-cookie mode: taint @@ -39,10 +58,7 @@ class SecureFlaskCookie(SimpleCodemod, SecureCookieMixin): - pattern: $SINK.set_cookie(...) - pattern-not: $SINK.set_cookie(..., secure=True, ..., httponly=True, ..., samesite="Lax", ...) - pattern-not: $SINK.set_cookie(..., secure=True, ..., httponly=True, ..., samesite="Strict", ...) - """ - - def on_result_found(self, original_node, updated_node): - new_args = self.replace_args( - original_node, self._choose_new_args(original_node) - ) - return self.update_arg_target(updated_node, new_args) + """ + ), + transformer=LibcstTransformerPipeline(SecureCookieTransformer), +) diff --git a/src/core_codemods/sonar/api.py b/src/core_codemods/sonar/api.py index 4833e675..5688770d 100644 --- a/src/core_codemods/sonar/api.py +++ b/src/core_codemods/sonar/api.py @@ -15,39 +15,53 @@ def origin(self): return "sonar" @classmethod - def from_core_codemod( + def from_core_codemod_with_multiple_rules( cls, name: str, other: CoreCodemod, - rule_id: str, - rule_name: str, + rules: list[ToolRule], transformer: BaseTransformerPipeline | None = None, ): - rule_url = sonar_url_from_id(rule_id) return SonarCodemod( metadata=Metadata( name=name, summary=other.summary, review_guidance=other._metadata.review_guidance, references=( - other.references + [Reference(url=rule_url, description=rule_name)] + other.references + + [Reference(url=tr.url or "", description=tr.name) for tr in rules] ), description=other.description, tool=ToolMetadata( name="Sonar", - rules=[ - ToolRule( - id=rule_id, - name=rule_name, - url=rule_url, - ) - ], + rules=rules, ), ), transformer=transformer if transformer else other.transformer, detector=SonarDetector(), default_extensions=other.default_extensions, - requested_rules=[rule_id], + requested_rules=[tr.id for tr in rules], + ) + + @classmethod + def from_core_codemod( + cls, + name: str, + other: CoreCodemod, + rule_id: str, + rule_name: str, + transformer: BaseTransformerPipeline | None = None, + ): + rule_url = sonar_url_from_id(rule_id) + rules = [ + ToolRule( + id=rule_id, + name=rule_name, + url=rule_url, + ), + ] + return cls.from_core_codemod_with_multiple_rules( + name, other, rules, transformer ) diff --git a/src/core_codemods/sonar/sonar_secure_cookie.py b/src/core_codemods/sonar/sonar_secure_cookie.py new file mode 100644 index 00000000..5f4cd1a9 --- /dev/null +++ b/src/core_codemods/sonar/sonar_secure_cookie.py @@ -0,0 +1,42 @@ +from codemodder.codemods.base_codemod import ToolRule +from codemodder.codemods.libcst_transformer import ( + LibcstResultTransformer, + LibcstTransformerPipeline, +) +from core_codemods.secure_cookie_mixin import SecureCookieMixin +from core_codemods.secure_flask_cookie import SecureFlaskCookie +from core_codemods.sonar.api import SonarCodemod + +rules = [ + ToolRule( + id="python:S3330", + name='Creating cookies without the "HttpOnly" flag is security-sensitive', + url="https://rules.sonarsource.com/python/RSPEC-3330/", + ), + ToolRule( + id="python:S2092", + name='Creating cookies without the "secure" flag is security-sensitive', + url="https://rules.sonarsource.com/python/RSPEC-2092/", + ), +] + + +class SonarSecureCookieTransformer(LibcstResultTransformer, SecureCookieMixin): + change_description = "Flask response `set_cookie` call should be called with `secure=True`, `httponly=True`, and `samesite='Lax'`." + + def leave_Call(self, original_node, updated_node): + if self.node_is_selected(original_node.func): + self.report_change(original_node) + new_args = self.replace_args( + original_node, self._choose_new_args(original_node) + ) + return self.update_arg_target(updated_node, new_args) + return updated_node + + +SonarSecureCookie = SonarCodemod.from_core_codemod_with_multiple_rules( + name="secure-cookie", + other=SecureFlaskCookie, + rules=rules, + transformer=LibcstTransformerPipeline(SonarSecureCookieTransformer), +) diff --git a/tests/codemods/sonar/test_sonar_secure_cookie.py b/tests/codemods/sonar/test_sonar_secure_cookie.py new file mode 100644 index 00000000..922fbc00 --- /dev/null +++ b/tests/codemods/sonar/test_sonar_secure_cookie.py @@ -0,0 +1,87 @@ +import json + +from codemodder.codemods.test import BaseSASTCodemodTest +from core_codemods.sonar.sonar_secure_cookie import SonarSecureCookie + + +class TestSonarSecureCookie(BaseSASTCodemodTest): + codemod = SonarSecureCookie + tool = "sonar" + + def test_name(self): + assert self.codemod.name == "secure-cookie" + + def test_simple(self, tmpdir): + input_code = """ + import flask + + response = flask.make_response() + var = "hello" + response.set_cookie("name", "value") + + response2 = flask.Response() + var = "hello" + response2.set_cookie("name", "value") + """ + expected = """ + import flask + + response = flask.make_response() + var = "hello" + response.set_cookie("name", "value", secure=True, httponly=True, samesite='Lax') + + response2 = flask.Response() + var = "hello" + response2.set_cookie("name", "value", secure=True, httponly=True, samesite='Lax') + """ + issues = { + "hotspots": [ + { + "component": "code.py", + "status": "TO_REVIEW", + "textRange": { + "startLine": 6, + "endLine": 6, + "startOffset": 0, + "endOffset": 19, + }, + "ruleKey": "python:S2092", + }, + { + "component": "code.py", + "status": "TO_REVIEW", + "textRange": { + "startLine": 10, + "endLine": 10, + "startOffset": 0, + "endOffset": 20, + }, + "ruleKey": "python:S2092", + }, + { + "component": "code.py", + "status": "TO_REVIEW", + "textRange": { + "startLine": 6, + "endLine": 6, + "startOffset": 0, + "endOffset": 19, + }, + "ruleKey": "python:S3330", + }, + { + "component": "code.py", + "status": "TO_REVIEW", + "textRange": { + "startLine": 10, + "endLine": 10, + "startOffset": 0, + "endOffset": 20, + }, + "ruleKey": "python:S3330", + }, + ], + } + self.run_and_assert( + tmpdir, input_code, expected, results=json.dumps(issues), num_changes=2 + ) diff --git a/tests/samples/secure_cookie.py b/tests/samples/secure_cookie.py new file mode 100644 index 00000000..e66bc986 --- /dev/null +++ b/tests/samples/secure_cookie.py @@ -0,0 +1,9 @@ +from flask import Flask, session, make_response + +app = Flask(__name__) + +@app.route('/') +def index(): + resp = make_response('Custom Cookie Set') + resp.set_cookie('custom_cookie', 'value') + return resp diff --git a/tests/samples/sonar_hotspots.json b/tests/samples/sonar_hotspots.json index ac52ee57..897220e9 100644 --- a/tests/samples/sonar_hotspots.json +++ b/tests/samples/sonar_hotspots.json @@ -5,6 +5,46 @@ "total": 4 }, "hotspots": [ + { + "key": "AZRvB_g13jBxJiUZnPHJ", + "component": "pixee_codemodder-python:secure_cookie.py", + "project": "pixee_codemodder-python", + "securityCategory": "insecure-conf", + "vulnerabilityProbability": "LOW", + "status": "TO_REVIEW", + "line": 8, + "message": "Make sure creating this cookie without the \"secure\" flag is safe.", + "creationDate": "2025-01-16T13:11:02+0100", + "updateDate": "2025-01-16T13:12:34+0100", + "textRange": { + "startLine": 8, + "endLine": 8, + "startOffset": 4, + "endOffset": 19 + }, + "flows": [], + "ruleKey": "python:S2092" + }, + { + "key": "AZRvB_g13jBxJiUZnPHI", + "component": "pixee_codemodder-python:secure_cookie.py", + "project": "pixee_codemodder-python", + "securityCategory": "others", + "vulnerabilityProbability": "LOW", + "status": "TO_REVIEW", + "line": 8, + "message": "Make sure creating this cookie without the \"HttpOnly\" flag is safe.", + "creationDate": "2025-01-16T13:11:02+0100", + "updateDate": "2025-01-16T13:12:34+0100", + "textRange": { + "startLine": 8, + "endLine": 8, + "startOffset": 4, + "endOffset": 19 + }, + "flows": [], + "ruleKey": "python:S3330" + }, { "key": "AY6fXn2rzaaymEtIucTd", "component": "pixee_codemodder-python:secure_random.py", diff --git a/tests/test_sonar_results.py b/tests/test_sonar_results.py index cb59e5a5..54227d4d 100644 --- a/tests/test_sonar_results.py +++ b/tests/test_sonar_results.py @@ -43,7 +43,7 @@ def test_parse_issues_json(): def test_parse_hotspots_json(): results = SonarResultSet.from_json(SAMPLE_DIR / "sonar_hotspots.json") - assert len(results) == 2 + assert len(results) == 4 def test_combined_json(tmpdir): @@ -54,7 +54,7 @@ def test_combined_json(tmpdir): ) results = SonarResultSet.from_json(Path(tmpdir).joinpath("combined.json")) - assert len(results) == 36 + assert len(results) == 38 def test_empty_issues(tmpdir, caplog):