diff --git a/integration_tests/base_test.py b/integration_tests/base_test.py index c894ab6ab..24a4f2800 100644 --- a/integration_tests/base_test.py +++ b/integration_tests/base_test.py @@ -83,6 +83,7 @@ def _assert_results_fields(self, results, output_path): assert len(results) == 1 result = results[0] assert result["codemod"] == self.codemod_wrapper.id + assert result["references"] == self.codemod_wrapper.references assert len(result["changeset"]) == self.num_changed_files # A codemod may change multiple files. For now we will diff --git a/src/codemodder/codemods/api/__init__.py b/src/codemodder/codemods/api/__init__.py index accbef433..c8a9a6613 100644 --- a/src/codemodder/codemods/api/__init__.py +++ b/src/codemodder/codemods/api/__init__.py @@ -69,6 +69,7 @@ def __init_subclass__(cls): cls.DESCRIPTION, # pylint: disable=no-member cls.NAME, # pylint: disable=no-member cls.REVIEW_GUIDANCE, # pylint: disable=no-member + cls.REFERENCES, # pylint: disable=no-member ) # This is a little bit hacky, but it also feels like the right solution? diff --git a/src/codemodder/codemods/base_codemod.py b/src/codemodder/codemods/base_codemod.py index 290f7ed68..c6393e353 100644 --- a/src/codemodder/codemods/base_codemod.py +++ b/src/codemodder/codemods/base_codemod.py @@ -1,4 +1,4 @@ -from dataclasses import dataclass +from dataclasses import dataclass, field from enum import Enum from pathlib import Path from typing import List, ClassVar @@ -22,6 +22,7 @@ class CodemodMetadata: DESCRIPTION: str # TODO: this field should be optional NAME: str REVIEW_GUIDANCE: ReviewGuidance + REFERENCES: list = field(default_factory=list) class BaseCodemod: diff --git a/src/codemodder/context.py b/src/codemodder/context.py index 26eeace93..70ac466fa 100644 --- a/src/codemodder/context.py +++ b/src/codemodder/context.py @@ -82,7 +82,7 @@ def compile_results(self, codemods: list[CodemodExecutorWrapper]): "codemod": codemod.id, "summary": codemod.summary, "description": codemod.description, - "references": [], + "references": codemod.references, "properties": {}, "failedFiles": [], "changeset": [change.to_json() for change in changeset], diff --git a/src/codemodder/executor.py b/src/codemodder/executor.py index 01eee72cc..47910b9f3 100644 --- a/src/codemodder/executor.py +++ b/src/codemodder/executor.py @@ -62,6 +62,10 @@ def description(self): def review_guidance(self): return self.METADATA.REVIEW_GUIDANCE.name.replace("_", " ").title() + @property + def references(self): + return self.METADATA.REFERENCES + @property def yaml_files(self): return [ diff --git a/src/codemodder/registry.py b/src/codemodder/registry.py index a903a8e4d..4a06997c0 100644 --- a/src/codemodder/registry.py +++ b/src/codemodder/registry.py @@ -61,7 +61,7 @@ def _validate_codemod(self, codemod): for k, v in asdict(codemod.METADATA).items(): if v is NotImplemented: raise NotImplementedError(f"METADATA.{k} not defined for {codemod}") - if not v: + if k != "REFERENCES" and not v: raise NotImplementedError( f"METADATA.{k} should not be None or empty for {codemod}" ) diff --git a/src/core_codemods/django_debug_flag_on.py b/src/core_codemods/django_debug_flag_on.py index de8766b8b..626ffe01b 100644 --- a/src/core_codemods/django_debug_flag_on.py +++ b/src/core_codemods/django_debug_flag_on.py @@ -18,6 +18,16 @@ class DjangoDebugFlagOn(SemgrepCodemod, Codemod): DESCRIPTION=("Flips django's debug flag if on."), NAME="django-debug-flag-on", REVIEW_GUIDANCE=ReviewGuidance.MERGE_AFTER_CURSORY_REVIEW, + REFERENCES=[ + { + "url": "https://owasp.org/www-project-top-ten/2017/A3_2017-Sensitive_Data_Exposure", + "description": "", + }, + { + "url": "https://docs.djangoproject.com/en/4.2/ref/settings/#std-setting-DEBUG", + "description": "", + }, + ], ) SUMMARY = CHANGE_DESCRIPTION = "Flip Django debug flag to off" YAML_FILES = [ diff --git a/src/core_codemods/django_session_cookie_secure_off.py b/src/core_codemods/django_session_cookie_secure_off.py index acfaa4eef..fa54e7dd6 100644 --- a/src/core_codemods/django_session_cookie_secure_off.py +++ b/src/core_codemods/django_session_cookie_secure_off.py @@ -18,6 +18,16 @@ class DjangoSessionCookieSecureOff(SemgrepCodemod, Codemod): DESCRIPTION=("Sets Django's `SESSION_COOKIE_SECURE` flag if off or missing."), NAME="django-session-cookie-secure-off", REVIEW_GUIDANCE=ReviewGuidance.MERGE_AFTER_REVIEW, + REFERENCES=[ + { + "url": "https://owasp.org/www-community/controls/SecureCookieAttribute", + "description": "", + }, + { + "url": "https://docs.djangoproject.com/en/4.2/ref/settings/#session-cookie-secure", + "description": "", + }, + ], ) SUMMARY = "Secure setting for Django `SESSION_COOKIE_SECURE` flag" CHANGE_DESCRIPTION = METADATA.DESCRIPTION diff --git a/src/core_codemods/enable_jinja2_autoescape.py b/src/core_codemods/enable_jinja2_autoescape.py index 8e8fe7989..8000b257d 100644 --- a/src/core_codemods/enable_jinja2_autoescape.py +++ b/src/core_codemods/enable_jinja2_autoescape.py @@ -8,6 +8,13 @@ class EnableJinja2Autoescape(SemgrepCodemod): REVIEW_GUIDANCE = ReviewGuidance.MERGE_AFTER_CURSORY_REVIEW SUMMARY = "Enable jinja2 autoescape" DESCRIPTION = "Makes the `autoescape` parameter to jinja2.Environment be `True`." + REFERENCES = [ + {"url": "https://owasp.org/www-community/attacks/xss/", "description": ""}, + { + "url": "https://jinja.palletsprojects.com/en/3.1.x/api/#autoescaping", + "description": "", + }, + ] @classmethod def rule(cls): diff --git a/src/core_codemods/fix_mutable_params.py b/src/core_codemods/fix_mutable_params.py index bd5cd5cd5..9bbae9922 100644 --- a/src/core_codemods/fix_mutable_params.py +++ b/src/core_codemods/fix_mutable_params.py @@ -10,7 +10,7 @@ class FixMutableParams(BaseCodemod): SUMMARY = "Replace Mutable Default Parameters" REVIEW_GUIDANCE = ReviewGuidance.MERGE_WITHOUT_REVIEW DESCRIPTION = "Replace mutable parameter with None" - + REFERENCES = [] _BUILTIN_TO_LITERAL = { "list": cst.List(elements=[]), "dict": cst.Dict(elements=[]), diff --git a/src/core_codemods/harden_pyyaml.py b/src/core_codemods/harden_pyyaml.py index 0fde850c0..2eed37a8a 100644 --- a/src/core_codemods/harden_pyyaml.py +++ b/src/core_codemods/harden_pyyaml.py @@ -7,6 +7,12 @@ class HardenPyyaml(SemgrepCodemod): REVIEW_GUIDANCE = ReviewGuidance.MERGE_WITHOUT_REVIEW SUMMARY = "Use SafeLoader when loading YAML" DESCRIPTION = "Ensures all calls to yaml.load use `SafeLoader`." + REFERENCES = [ + { + "url": "https://owasp.org/www-community/vulnerabilities/Deserialization_of_untrusted_data", + "description": "", + } + ] @classmethod def rule(cls): diff --git a/src/core_codemods/harden_ruamel.py b/src/core_codemods/harden_ruamel.py index 86c9a7c3f..e7e6d373b 100644 --- a/src/core_codemods/harden_ruamel.py +++ b/src/core_codemods/harden_ruamel.py @@ -8,6 +8,12 @@ class HardenRuamel(SemgrepCodemod): REVIEW_GUIDANCE = ReviewGuidance.MERGE_WITHOUT_REVIEW SUMMARY = "Use safe YAML loading in ruamel.yaml" DESCRIPTION = "Ensures all unsafe calls to ruamel.yaml.YAML use `typ='safe'`." + REFERENCES = [ + { + "url": "https://owasp.org/www-community/vulnerabilities/Deserialization_of_untrusted_data", + "description": "", + } + ] @classmethod def rule(cls): diff --git a/src/core_codemods/https_connection.py b/src/core_codemods/https_connection.py index 2ba92ee23..bec1f4f7b 100644 --- a/src/core_codemods/https_connection.py +++ b/src/core_codemods/https_connection.py @@ -25,6 +25,16 @@ class HTTPSConnection(BaseCodemod, Codemod): DESCRIPTION=("Enforce HTTPS connection"), NAME="https-connection", REVIEW_GUIDANCE=ReviewGuidance.MERGE_WITHOUT_REVIEW, + REFERENCES=[ + { + "url": "https://owasp.org/www-community/vulnerabilities/Insecure_Transport", + "description": "", + }, + { + "url": "https://urllib3.readthedocs.io/en/stable/reference/urllib3.connectionpool.html#urllib3.HTTPConnectionPool", + "description": "", + }, + ], ) CHANGE_DESCRIPTION = "Enforce HTTPS connection" SUMMARY = "Changes HTTPConnectionPool to HTTPSConnectionPool to enforce secure connection." diff --git a/src/core_codemods/jwt_decode_verify.py b/src/core_codemods/jwt_decode_verify.py index f1e4f1451..44c0198c4 100644 --- a/src/core_codemods/jwt_decode_verify.py +++ b/src/core_codemods/jwt_decode_verify.py @@ -10,6 +10,13 @@ class JwtDecodeVerify(SemgrepCodemod): REVIEW_GUIDANCE = ReviewGuidance.MERGE_WITHOUT_REVIEW SUMMARY = "Enable all verifications in `jwt.decode` call." DESCRIPTION = "Makes any of the multiple `verify` parameters to a `jwt.decode` call be `True`." + REFERENCES = [ + {"url": "https://pyjwt.readthedocs.io/en/stable/api.html", "description": ""}, + { + "url": "https://owasp.org/www-project-web-security-testing-guide/latest/4-Web_Application_Security_Testing/06-Session_Management_Testing/10-Testing_JSON_Web_Tokens", + "description": "", + }, + ] @classmethod def rule(cls): diff --git a/src/core_codemods/limit_readline.py b/src/core_codemods/limit_readline.py index e0dc77dc5..158787e7d 100644 --- a/src/core_codemods/limit_readline.py +++ b/src/core_codemods/limit_readline.py @@ -11,6 +11,9 @@ class LimitReadline(SemgrepCodemod): REVIEW_GUIDANCE = ReviewGuidance.MERGE_AFTER_CURSORY_REVIEW SUMMARY = "Limit the size of readline() calls" DESCRIPTION = "Adds a size limit argument to readline() calls." + REFERENCES = [ + {"url": "https://cwe.mitre.org/data/definitions/400.html", "description": ""} + ] @classmethod def rule(cls): diff --git a/src/core_codemods/lxml_safe_parser_defaults.py b/src/core_codemods/lxml_safe_parser_defaults.py index de2d7759d..4479d9cc7 100644 --- a/src/core_codemods/lxml_safe_parser_defaults.py +++ b/src/core_codemods/lxml_safe_parser_defaults.py @@ -8,6 +8,20 @@ class LxmlSafeParserDefaults(SemgrepCodemod): REVIEW_GUIDANCE = ReviewGuidance.MERGE_WITHOUT_REVIEW SUMMARY = "Use Safe Defaults for lxml Parsers" DESCRIPTION = "Replace lxml parser parameters with safe defaults" + REFERENCES = [ + { + "url": "https://lxml.de/apidoc/lxml.etree.html#lxml.etree.XMLParser", + "description": "", + }, + { + "url": "https://owasp.org/www-community/vulnerabilities/XML_External_Entity_(XXE)_Processing", + "description": "", + }, + { + "url": "https://cheatsheetseries.owasp.org/cheatsheets/XML_External_Entity_Prevention_Cheat_Sheet.html", + "description": "", + }, + ] @classmethod def rule(cls): diff --git a/src/core_codemods/lxml_safe_parsing.py b/src/core_codemods/lxml_safe_parsing.py index 6ddd817b0..dc5ead322 100644 --- a/src/core_codemods/lxml_safe_parsing.py +++ b/src/core_codemods/lxml_safe_parsing.py @@ -10,6 +10,20 @@ class LxmlSafeParsing(SemgrepCodemod): DESCRIPTION = ( "Call `lxml.etree.parse` and `lxml.etree.fromstring` with a safe parser" ) + REFERENCES = [ + { + "url": "https://lxml.de/apidoc/lxml.etree.html#lxml.etree.XMLParser", + "description": "", + }, + { + "url": "https://owasp.org/www-community/vulnerabilities/XML_External_Entity_(XXE)_Processing", + "description": "", + }, + { + "url": "https://cheatsheetseries.owasp.org/cheatsheets/XML_External_Entity_Prevention_Cheat_Sheet.html", + "description": "", + }, + ] @classmethod def rule(cls): diff --git a/src/core_codemods/order_imports.py b/src/core_codemods/order_imports.py index c0ddfa941..4382d17d2 100644 --- a/src/core_codemods/order_imports.py +++ b/src/core_codemods/order_imports.py @@ -18,6 +18,7 @@ class OrderImports(BaseCodemod, Codemod): DESCRIPTION=("Formats and order imports by categories"), NAME="order-imports", REVIEW_GUIDANCE=ReviewGuidance.MERGE_WITHOUT_REVIEW, + REFERENCES=[], ) SUMMARY = "Order imports by categories" CHANGE_DESCRIPTION = "Ordered and formatted import block below this line" diff --git a/src/core_codemods/process_creation_sandbox.py b/src/core_codemods/process_creation_sandbox.py index 84d46ffd3..b5a88281d 100644 --- a/src/core_codemods/process_creation_sandbox.py +++ b/src/core_codemods/process_creation_sandbox.py @@ -10,6 +10,16 @@ class ProcessSandbox(SemgrepCodemod): DESCRIPTION = ( "Replaces subprocess.{func} with more secure safe_command library functions." ) + REFERENCES = [ + { + "url": "https://github.com/pixee/python-security/blob/main/src/security/safe_command/api.py", + "description": "", + }, + { + "url": "https://cheatsheetseries.owasp.org/cheatsheets/OS_Command_Injection_Defense_Cheat_Sheet.html", + "description": "", + }, + ] @classmethod def rule(cls): diff --git a/src/core_codemods/remove_unnecessary_f_str.py b/src/core_codemods/remove_unnecessary_f_str.py index 58fc65e41..c85c49946 100644 --- a/src/core_codemods/remove_unnecessary_f_str.py +++ b/src/core_codemods/remove_unnecessary_f_str.py @@ -13,6 +13,12 @@ class RemoveUnnecessaryFStr(BaseCodemod, UnnecessaryFormatString): REVIEW_GUIDANCE = ReviewGuidance.MERGE_WITHOUT_REVIEW SUMMARY = "Remove unnecessary f-strings" DESCRIPTION = UnnecessaryFormatString.DESCRIPTION + REFERENCES = [ + { + "url": "https://github.com/Instagram/LibCST/blob/main/libcst/codemod/commands/unnecessary_format_string.py", + "description": "", + } + ] def __init__(self, codemod_context: CodemodContext, *codemod_args): UnnecessaryFormatString.__init__(self, codemod_context) diff --git a/src/core_codemods/remove_unused_imports.py b/src/core_codemods/remove_unused_imports.py index 5744043dd..93b33e5d2 100644 --- a/src/core_codemods/remove_unused_imports.py +++ b/src/core_codemods/remove_unused_imports.py @@ -28,6 +28,7 @@ class RemoveUnusedImports(BaseCodemod, Codemod): DESCRIPTION=("Remove unused imports from a module."), NAME="unused-imports", REVIEW_GUIDANCE=ReviewGuidance.MERGE_WITHOUT_REVIEW, + REFERENCES=[], ) SUMMARY = "Remove unused imports from a module" CHANGE_DESCRIPTION = "Unused import." diff --git a/src/core_codemods/requests_verify.py b/src/core_codemods/requests_verify.py index c01d45943..89b4af3b0 100644 --- a/src/core_codemods/requests_verify.py +++ b/src/core_codemods/requests_verify.py @@ -10,6 +10,13 @@ class RequestsVerify(SemgrepCodemod): DESCRIPTION = ( "Makes any calls to requests.{func} with `verify=False` to `verify=True`" ) + REFERENCES = [ + {"url": "https://requests.readthedocs.io/en/latest/api/", "description": ""}, + { + "url": "https://owasp.org/www-community/attacks/Manipulator-in-the-middle_attack", + "description": "", + }, + ] @classmethod def rule(cls): diff --git a/src/core_codemods/secure_random.py b/src/core_codemods/secure_random.py index 7aebf8629..c76747dda 100644 --- a/src/core_codemods/secure_random.py +++ b/src/core_codemods/secure_random.py @@ -7,6 +7,13 @@ class SecureRandom(SemgrepCodemod): REVIEW_GUIDANCE = ReviewGuidance.MERGE_WITHOUT_REVIEW SUMMARY = "Use secrets.SystemRandom() instead of random" DESCRIPTION = "Replaces random.{func} with more secure secrets library functions." + REFERENCES = [ + { + "url": "https://owasp.org/www-community/vulnerabilities/Insecure_Randomness", + "description": "", + }, + {"url": "https://docs.python.org/3/library/random.html", "description": ""}, + ] @classmethod def rule(cls): diff --git a/src/core_codemods/tempfile_mktemp.py b/src/core_codemods/tempfile_mktemp.py index 26d92d5e8..563ce016b 100644 --- a/src/core_codemods/tempfile_mktemp.py +++ b/src/core_codemods/tempfile_mktemp.py @@ -7,6 +7,12 @@ class TempfileMktemp(SemgrepCodemod): REVIEW_GUIDANCE = ReviewGuidance.MERGE_WITHOUT_REVIEW SUMMARY = "Use `tempfile.mkstemp` instead of `tempfile.mktemp`" DESCRIPTION = "Replaces `tempfile.mktemp` with `tempfile.mkstemp`." + REFERENCES = [ + { + "url": "https://docs.python.org/3/library/tempfile.html#tempfile.mktemp", + "description": "", + } + ] @classmethod def rule(cls): diff --git a/src/core_codemods/upgrade_sslcontext_minimum_version.py b/src/core_codemods/upgrade_sslcontext_minimum_version.py index cd8763dfb..52a006a41 100644 --- a/src/core_codemods/upgrade_sslcontext_minimum_version.py +++ b/src/core_codemods/upgrade_sslcontext_minimum_version.py @@ -7,6 +7,17 @@ class UpgradeSSLContextMinimumVersion(SemgrepCodemod): REVIEW_GUIDANCE = ReviewGuidance.MERGE_WITHOUT_REVIEW SUMMARY = "Upgrade minimum SSL/TLS version for SSLContext" DESCRIPTION = "Replaces minimum SSL/TLS version for SSLContext" + REFERENCES = [ + { + "url": "https://docs.python.org/3/library/ssl.html#security-considerations", + "description": "", + }, + {"url": "https://datatracker.ietf.org/doc/rfc8996/", "description": ""}, + { + "url": "https://www.digicert.com/blog/depreciating-tls-1-0-and-1-1", + "description": "", + }, + ] @classmethod def rule(cls): diff --git a/src/core_codemods/upgrade_sslcontext_tls.py b/src/core_codemods/upgrade_sslcontext_tls.py index a2e42182c..333eb0f1c 100644 --- a/src/core_codemods/upgrade_sslcontext_tls.py +++ b/src/core_codemods/upgrade_sslcontext_tls.py @@ -16,6 +16,17 @@ class UpgradeSSLContextTLS(SemgrepCodemod, BaseTransformer): DESCRIPTION="Replaces known insecure TLS/SSL protocol versions in SSLContext with secure ones", NAME="upgrade-sslcontext-tls", REVIEW_GUIDANCE=ReviewGuidance.MERGE_AFTER_CURSORY_REVIEW, + REFERENCES=[ + { + "url": "https://docs.python.org/3/library/ssl.html#security-considerations", + "description": "", + }, + {"url": "https://datatracker.ietf.org/doc/rfc8996/", "description": ""}, + { + "url": "https://www.digicert.com/blog/depreciating-tls-1-0-and-1-1", + "description": "", + }, + ], ) SUMMARY = "Replace known insecure TLS/SSL protocol versions in SSLContext with secure ones" CHANGE_DESCRIPTION = "Upgrade to use a safe version of TLS in SSLContext" diff --git a/src/core_codemods/url_sandbox.py b/src/core_codemods/url_sandbox.py index c7f7c8f16..3a4b60cd6 100644 --- a/src/core_codemods/url_sandbox.py +++ b/src/core_codemods/url_sandbox.py @@ -29,6 +29,25 @@ class UrlSandbox(SemgrepCodemod, Codemod): ), NAME="url-sandbox", REVIEW_GUIDANCE=ReviewGuidance.MERGE_AFTER_CURSORY_REVIEW, + REFERENCES=[ + { + "url": "https://github.com/pixee/python-security/blob/main/src/security/safe_requests/api.py", + "description": "", + }, + {"url": "https://portswigger.net/web-security/ssrf", "description": ""}, + { + "url": "https://cheatsheetseries.owasp.org/cheatsheets/Server_Side_Request_Forgery_Prevention_Cheat_Sheet.html", + "description": "", + }, + { + "url": "https://www.rapid7.com/blog/post/2021/11/23/owasp-top-10-deep-dive-defending-against-server-side-request-forgery/", + "description": "", + }, + { + "url": "https://blog.assetnote.io/2021/01/13/blind-ssrf-chains/", + "description": "", + }, + ], ) SUMMARY = "Ensure that requests are made safely." CHANGE_DESCRIPTION = "Switch use of requests for security.safe_requests" diff --git a/src/core_codemods/use_walrus_if.py b/src/core_codemods/use_walrus_if.py index 6508d2772..28b15fe3f 100644 --- a/src/core_codemods/use_walrus_if.py +++ b/src/core_codemods/use_walrus_if.py @@ -20,6 +20,12 @@ class UseWalrusIf(SemgrepCodemod): DESCRIPTION = ( "Replaces multiple expressions involving `if` operator with 'walrus' operator" ) + REFERENCES = [ + { + "url": "https://docs.python.org/3/whatsnew/3.8.html#assignment-expressions", + "description": "", + } + ] @classmethod def rule(cls): diff --git a/src/core_codemods/with_threading_lock.py b/src/core_codemods/with_threading_lock.py index 349bb48fc..00e6365ee 100644 --- a/src/core_codemods/with_threading_lock.py +++ b/src/core_codemods/with_threading_lock.py @@ -8,6 +8,16 @@ class WithThreadingLock(SemgrepCodemod): SUMMARY = "Replace deprecated usage of threading lock classes as context managers" REVIEW_GUIDANCE = ReviewGuidance.MERGE_AFTER_CURSORY_REVIEW DESCRIPTION = "Separates threading lock instantiation and call with `with` statement into two steps." + REFERENCES = [ + { + "url": "https://pylint.pycqa.org/en/latest/user_guide/messages/warning/useless-with-lock.", + "description": "", + }, + { + "url": "https://docs.python.org/3/library/threading.html#using-locks-conditions-and-semaphores-in-the-with-statement", + "description": "", + }, + ] @classmethod def rule(cls): diff --git a/src/scripts/generate_docs.py b/src/scripts/generate_docs.py index 388d233f2..3458d4c22 100644 --- a/src/scripts/generate_docs.py +++ b/src/scripts/generate_docs.py @@ -58,10 +58,10 @@ class DocMetadata: importance="High", guidance_explained="We believe this change is safe, effective, and protects your code against very serious security attacks.", ), - "order-imports": DocMetadata( - importance="Low", - guidance_explained="We believe this codemod is safe and will not cause any issues. It is important to note that importing modules may have side-effects that alter the behavior, even if unused, but we believe those cases are rare enough to be safe.", - ), + # "order-imports": DocMetadata( + # importance="Low", + # guidance_explained="", + # ), "sandbox-process-creation": DocMetadata( importance="High", guidance_explained="We believe this change is safe and effective. The behavior of sandboxing `subprocess.run` and `subprocess.call` calls will only throw `SecurityException` if they see behavior involved in malicious code execution, which is extremely unlikely to happen in normal operation.",