sonar url-sandbox (#445)

clavedeluna · web-flow · commit 1460393f06cd · 2024-04-04T18:02:58.000Z
add sonar url sandbox codemod
diff --git a/integration_tests/test_sonar_url_sandbox.py b/integration_tests/test_sonar_url_sandbox.py
@@ -0,0 +1,16 @@
+from codemodder.codemods.test import SonarIntegrationTest
+from core_codemods.sonar.sonar_url_sandbox import SonarUrlSandbox
+from core_codemods.url_sandbox import UrlSandboxTransformer
+
+
+class TestSonarUrlSandbox(SonarIntegrationTest):
+    codemod = SonarUrlSandbox
+    code_path = "tests/samples/flask_request.py"
+    replacement_lines = [
+        (1, "from flask import Flask, request\n"),
+        (2, "from security import safe_requests\n"),
+        (10, "    safe_requests.get(url)\n"),
+    ]
+    expected_diff = '--- \n+++ \n@@ -1,5 +1,5 @@\n-import requests\n from flask import Flask, request\n+from security import safe_requests\n \n app = Flask(__name__)\n \n@@ -7,4 +7,4 @@\n @app.route("/example")\n def example():\n     url = request.args["url"]\n-    requests.get(url)\n+    safe_requests.get(url)\n'
+    expected_line_change = "10"
+    change_description = UrlSandboxTransformer.change_description
diff --git a/integration_tests/test_url_sandbox.py b/integration_tests/test_url_sandbox.py
@@ -1,6 +1,6 @@
 from codemodder.codemods.test import BaseIntegrationTest
 from codemodder.dependency import Security
-from core_codemods.url_sandbox import UrlSandbox
+from core_codemods.url_sandbox import UrlSandbox, UrlSandboxTransformer
 
 
 class TestUrlSandbox(BaseIntegrationTest):
@@ -32,7 +32,7 @@ class TestUrlSandbox(BaseIntegrationTest):
 """
 
     expected_line_change = "5"
-    change_description = UrlSandbox.change_description
+    change_description = UrlSandboxTransformer.change_description
     num_changed_files = 2
 
     requirements_file_name = "requirements.txt"
diff --git a/src/codemodder/scripts/generate_docs.py b/src/codemodder/scripts/generate_docs.py
@@ -339,6 +339,11 @@ class DocMetadata:
         guidance_explained="This change is generally safe and will prevent deserialization vulnerabilities.",
         need_sarif="Yes (DefectDojo)",
     ),
+    "url-sandbox-S5144": DocMetadata(
+        importance=CORE_METADATA["url-sandbox"].importance,
+        guidance_explained=CORE_METADATA["url-sandbox"].guidance_explained,
+        need_sarif="Yes (Sonar)",
+    ),
 }
 
 
diff --git a/src/core_codemods/__init__.py b/src/core_codemods/__init__.py
@@ -64,6 +64,7 @@
 )
 from .sonar.sonar_secure_random import SonarSecureRandom
 from .sonar.sonar_tempfile_mktemp import SonarTempfileMktemp
+from .sonar.sonar_url_sandbox import SonarUrlSandbox
 from .sql_parameterization import SQLQueryParameterization
 from .str_concat_in_seq_literal import StrConcatInSeqLiteral
 from .subprocess_shell_false import SubprocessShellFalse
@@ -156,6 +157,7 @@
         SonarTempfileMktemp,
         SonarSecureRandom,
         SonarEnableJinja2Autoescape,
+        SonarUrlSandbox,
     ],
 )
 
diff --git a/src/core_codemods/sonar/sonar_url_sandbox.py b/src/core_codemods/sonar/sonar_url_sandbox.py
@@ -0,0 +1,10 @@
+from codemodder.codemods.sonar import SonarCodemod
+from core_codemods.url_sandbox import UrlSandbox
+
+SonarUrlSandbox = SonarCodemod.from_core_codemod(
+    name="url-sandbox-S5144",
+    other=UrlSandbox,
+    rule_id="pythonsecurity:S5144",
+    rule_name="Server-side requests should not be vulnerable to forging attacks",
+    rule_url="https://rules.sonarsource.com/python/type/Vulnerability/RSPEC-5144/",
+)
diff --git a/src/core_codemods/url_sandbox.py b/src/core_codemods/url_sandbox.py
@@ -7,57 +7,26 @@
 from libcst.metadata import PositionProvider, ScopeProvider
 
 from codemodder.codemods.base_visitor import UtilsMixin
+from codemodder.codemods.libcst_transformer import (
+    LibcstResultTransformer,
+    LibcstTransformerPipeline,
+)
+from codemodder.codemods.semgrep import SemgrepRuleDetector
 from codemodder.codemods.transformations.remove_unused_imports import (
     RemoveUnusedImportsCodemod,
 )
 from codemodder.codemods.utils import ReplaceNodes
 from codemodder.codetf import Change
 from codemodder.dependency import Security
 from codemodder.file_context import FileContext
-from core_codemods.api import Metadata, Reference, ReviewGuidance, SimpleCodemod
+from core_codemods.api import CoreCodemod, Metadata, Reference, ReviewGuidance
 
 replacement_import = "safe_requests"
 
 
-class UrlSandbox(SimpleCodemod):
-    metadata = Metadata(
-        name="url-sandbox",
-        summary="Sandbox URL Creation",
-        review_guidance=ReviewGuidance.MERGE_AFTER_CURSORY_REVIEW,
-        references=[
-            Reference(
-                url="https://github.com/pixee/python-security/blob/main/src/security/safe_requests/api.py"
-            ),
-            Reference(url="https://portswigger.net/web-security/ssrf"),
-            Reference(
-                url="https://cheatsheetseries.owasp.org/cheatsheets/Server_Side_Request_Forgery_Prevention_Cheat_Sheet.html"
-            ),
-            Reference(
-                url="https://www.rapid7.com/blog/post/2021/11/23/owasp-top-10-deep-dive-defending-against-server-side-request-forgery/"
-            ),
-            Reference(url="https://blog.assetnote.io/2021/01/13/blind-ssrf-chains/"),
-        ],
-    )
+class UrlSandboxTransformer(LibcstResultTransformer):
     change_description = "Switch use of requests for security.safe_requests"
-
-    detector_pattern = """
-    rules:
-      - id: url-sandbox
-        message: Unbounded URL creation
-        severity: WARNING
-        languages:
-          - python
-        pattern-either:
-          - patterns:
-            - pattern: requests.get(...)
-            - pattern-not: requests.get("...")
-            - pattern-inside: |
-                import requests
-                ...
-            """
-
     METADATA_DEPENDENCIES = (PositionProvider, ScopeProvider)
-
     adds_dependency = True
 
     def transform_module_impl(self, tree: cst.Module) -> cst.Module:
@@ -111,7 +80,7 @@ def __init__(
         )
 
     def leave_Call(self, original_node: cst.Call):
-        if not (self.node_is_selected(original_node)):
+        if not self.node_is_selected(original_node):
             return
 
         line_number = self.node_position(original_node).start.line
@@ -139,7 +108,7 @@ def leave_Call(self, original_node: cst.Call):
                         self.changes_in_file.append(
                             Change(
                                 lineNumber=line_number,
-                                description=UrlSandbox.change_description,
+                                description=UrlSandboxTransformer.change_description,
                             )
                         )
 
@@ -156,7 +125,7 @@ def leave_Call(self, original_node: cst.Call):
                 self.changes_in_file.append(
                     Change(
                         lineNumber=line_number,
-                        description=UrlSandbox.change_description,
+                        description=UrlSandboxTransformer.change_description,
                     )
                 )
 
@@ -175,3 +144,43 @@ def find_single_assignment(self, node: CSTNode) -> Optional[CSTNode]:
         if len(assignments) == 1:
             return next(iter(assignments)).node
         return None
+
+
+UrlSandbox = CoreCodemod(
+    metadata=Metadata(
+        name="url-sandbox",
+        summary="Sandbox URL Creation",
+        review_guidance=ReviewGuidance.MERGE_AFTER_CURSORY_REVIEW,
+        references=[
+            Reference(
+                url="https://github.com/pixee/python-security/blob/main/src/security/safe_requests/api.py"
+            ),
+            Reference(url="https://portswigger.net/web-security/ssrf"),
+            Reference(
+                url="https://cheatsheetseries.owasp.org/cheatsheets/Server_Side_Request_Forgery_Prevention_Cheat_Sheet.html"
+            ),
+            Reference(
+                url="https://www.rapid7.com/blog/post/2021/11/23/owasp-top-10-deep-dive-defending-against-server-side-request-forgery/"
+            ),
+            Reference(url="https://blog.assetnote.io/2021/01/13/blind-ssrf-chains/"),
+        ],
+    ),
+    detector=SemgrepRuleDetector(
+        """
+    rules:
+      - id: url-sandbox
+        message: Unbounded URL creation
+        severity: WARNING
+        languages:
+          - python
+        pattern-either:
+          - patterns:
+            - pattern: requests.get(...)
+            - pattern-not: requests.get("...")
+            - pattern-inside: |
+                import requests
+                ...
+            """
+    ),
+    transformer=LibcstTransformerPipeline(UrlSandboxTransformer),
+)
diff --git a/tests/codemods/test_sonar_url_sandbox.py b/tests/codemods/test_sonar_url_sandbox.py
@@ -0,0 +1,54 @@
+import json
+
+from codemodder.codemods.test import BaseSASTCodemodTest
+from core_codemods.sonar.sonar_url_sandbox import SonarUrlSandbox
+
+
+class TestSonarUrlSandbox(BaseSASTCodemodTest):
+    codemod = SonarUrlSandbox
+    tool = "sonar"
+
+    def test_name(self):
+        assert self.codemod.name == "url-sandbox-S5144"
+
+    def test_simple(self, tmpdir):
+        input_code = """
+        import requests
+        from flask import Flask, request
+        
+        app = Flask(__name__)
+        
+        
+        @app.route("/example")
+        def example():
+            url = request.args["url"]
+            requests.get(url)
+        """
+        expected = """
+        from flask import Flask, request
+        from security import safe_requests
+        
+        app = Flask(__name__)
+        
+        
+        @app.route("/example")
+        def example():
+            url = request.args["url"]
+            safe_requests.get(url)
+        """
+        issues = {
+            "issues": [
+                {
+                    "rule": "pythonsecurity:S5144",
+                    "status": "OPEN",
+                    "component": "code.py",
+                    "textRange": {
+                        "startLine": 11,
+                        "endLine": 11,
+                        "startOffset": 4,
+                        "endOffset": 21,
+                    },
+                }
+            ]
+        }
+        self.run_and_assert(tmpdir, input_code, expected, results=json.dumps(issues))
diff --git a/tests/samples/flask_request.py b/tests/samples/flask_request.py
@@ -0,0 +1,10 @@
+import requests
+from flask import Flask, request
+
+app = Flask(__name__)
+
+
+@app.route("/example")
+def example():
+    url = request.args["url"]
+    requests.get(url)
diff --git a/tests/samples/sonar_issues.json b/tests/samples/sonar_issues.json
@@ -1,5 +1,5 @@
 {
-  "total":37,
+  "total":38,
   "p":1,
   "ps":500,
   "paging":{
@@ -1608,6 +1608,78 @@
           "severity":"HIGH"
         }
       ]
+    },
+    {
+      "key": "AY6o8XTbdw-0_54hqn7F",
+      "rule": "pythonsecurity:S5144",
+      "severity": "MAJOR",
+      "component": "pixee_codemodder-python:flask_request.py",
+      "project": "pixee_codemodder-python",
+      "line": 10,
+      "hash": "c864c19608fe39f9e580a5deefb67381",
+      "textRange": {
+        "startLine": 10,
+        "endLine": 10,
+        "startOffset": 4,
+        "endOffset": 21
+      },
+      "flows": [
+        {
+          "locations": [
+            {
+              "component": "pixee_codemodder-python:flask_request.py",
+              "textRange": {
+                "startLine": 10,
+                "endLine": 10,
+                "startOffset": 4,
+                "endOffset": 21
+              },
+              "msg": "Sink: this invocation is not safe; a malicious value can be used as argument"
+            },
+            {
+              "component": "pixee_codemodder-python:flask_request.py",
+              "textRange": {
+                "startLine": 9,
+                "endLine": 9,
+                "startOffset": 4,
+                "endOffset": 29
+              },
+              "msg": "A malicious value can be assigned to variable ‘url’"
+            },
+            {
+              "component": "pixee_codemodder-python:flask_request.py",
+              "textRange": {
+                "startLine": 9,
+                "endLine": 9,
+                "startOffset": 10,
+                "endOffset": 29
+              },
+              "msg": "Source: a user can craft an HTTP request with malicious content"
+            }
+          ]
+        }
+      ],
+      "status": "OPEN",
+      "message": "Change this code to not construct the URL from user-controlled data.",
+      "effort": "30min",
+      "debt": "30min",
+      "assignee": "clavedeluna@github",
+      "tags": [
+        "cwe"
+      ],
+      "creationDate": "2024-04-04T13:49:07+0200",
+      "updateDate": "2024-04-04T13:49:25+0200",
+      "type": "VULNERABILITY",
+      "organization": "pixee",
+      "pullRequest": "445",
+      "cleanCodeAttribute": "COMPLETE",
+      "cleanCodeAttributeCategory": "INTENTIONAL",
+      "impacts": [
+        {
+          "softwareQuality": "SECURITY",
+          "severity": "MEDIUM"
+        }
+      ]
     }
   ],
   "components":[

Original file line number	Diff line number	Diff line change
`@@ -64,6 +64,7 @@`
`64`	`64`	`)`
`65`	`65`	`from .sonar.sonar_secure_random import SonarSecureRandom`
`66`	`66`	`from .sonar.sonar_tempfile_mktemp import SonarTempfileMktemp`
	`67`	`+from .sonar.sonar_url_sandbox import SonarUrlSandbox`
`67`	`68`	`from .sql_parameterization import SQLQueryParameterization`
`68`	`69`	`from .str_concat_in_seq_literal import StrConcatInSeqLiteral`
`69`	`70`	`from .subprocess_shell_false import SubprocessShellFalse`
`@@ -156,6 +157,7 @@`
`156`	`157`	`SonarTempfileMktemp,`
`157`	`158`	`SonarSecureRandom,`
`158`	`159`	`SonarEnableJinja2Autoescape,`
	`160`	`+ SonarUrlSandbox,`
`159`	`161`	`],`
`160`	`162`	`)`
`161`	`163`