New Codemod: Sonar Flask Secure Cookie (#969)

* Initial version of SonarSecureCookie

* Added unit test

* Added integration test

* Fixed missing file for integration test

* Fixed tests and refactoring

Author: andrecsilva

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

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

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"
 
 

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 = [

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,
     ],
 )
 

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),
+)

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
         )
 
 

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),
+)

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
+        )

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