From dad7a58773f2cd65f7e316c8749ff79062fde52d Mon Sep 17 00:00:00 2001 From: Daniel D'Avella Date: Tue, 14 May 2024 10:19:18 -0400 Subject: [PATCH] Add new Python codemods --- ...o_python_avoid-insecure-deserialization.md | 66 +++++++++++ ...ectdojo_python_django-secure-set-cookie.md | 39 +++++++ ...ee_python_break-or-continue-out-of-loop.md | 36 ++++++ ...ee_python_combine-isinstance-issubclass.md | 39 +++++++ ...ee_python_disable-graphql-introspection.md | 61 ++++++++++ .../pixee_python_django-debug-flag-on.md | 2 +- ..._python_django-model-without-dunder-str.md | 61 ++++++++++ .../pixee_python_django-receiver-on-top.md | 2 +- ...python_django-session-cookie-secure-off.md | 2 +- .../python/pixee_python_fix-assert-tuple.md | 2 +- .../pixee_python_fix-dataclass-defaults.md | 45 ++++++++ ..._python_fix-deprecated-abstractproperty.md | 8 +- .../python/pixee_python_fix-float-equality.md | 41 +++++++ .../python/pixee_python_fix-hasattr-call.md | 40 +++++++ .../python/pixee_python_fix-math-isclose.md | 40 +++++++ .../pixee_python_fix-missing-self-or-cls.md | 42 +++++++ .../python/pixee_python_fix-mutable-params.md | 2 +- .../python/pixee_python_harden-pickle-load.md | 45 ++++++++ .../python/pixee_python_harden-pyyaml.md | 13 ++- .../python/pixee_python_lazy-logging.md | 2 +- .../python/pixee_python_numpy-nan-equality.md | 2 +- .../pixee_python_remove-debug-breakpoint.md | 4 +- ..._python_str-concat-in-sequence-literals.md | 2 +- ...hon_break-or-continue-out-of-loop-S1716.md | 39 +++++++ ...hon_disable-graphql-introspection-S6786.md | 64 ++++++++++ ..._python_django-json-response-type-S5131.md | 4 +- ...n_django-model-without-dunder-str-S6554.md | 64 ++++++++++ ...nar_python_django-receiver-on-top-S6552.md | 6 +- ...r_python_enable-jinja2-autoescape-S5247.md | 44 +++++++ ...ar_python_exception-without-raise-S3984.md | 4 +- .../sonar_python_fix-assert-tuple-S5905.md | 6 +- .../sonar_python_fix-float-equality-S1244.md | 44 +++++++ .../sonar_python_fix-math-isclose-S6727.md | 43 +++++++ ...ar_python_fix-missing-self-or-cls-S5719.md | 44 +++++++ ...r_python_flask-json-response-type-S5131.md | 4 +- .../sonar_python_jwt-decode-verify-S5659.md | 46 ++++++++ ...on_literal-or-new-object-identity-S5796.md | 4 +- .../sonar_python_numpy-nan-equality-S6725.md | 6 +- ...remove-assertion-in-pytest-raises-S5915.md | 4 +- .../sonar_python_secure-random-S2245.md | 46 ++++++++ .../sonar_python_secure-tempfile-S5445.md | 41 +++++++ ...sonar_python_sql-parameterization-S3649.md | 46 ++++++++ .../python/sonar_python_url-sandbox-S5144.md | 109 ++++++++++++++++++ 43 files changed, 1228 insertions(+), 36 deletions(-) create mode 100644 docs/codemods/python/defectdojo_python_avoid-insecure-deserialization.md create mode 100644 docs/codemods/python/defectdojo_python_django-secure-set-cookie.md create mode 100644 docs/codemods/python/pixee_python_break-or-continue-out-of-loop.md create mode 100644 docs/codemods/python/pixee_python_combine-isinstance-issubclass.md create mode 100644 docs/codemods/python/pixee_python_disable-graphql-introspection.md create mode 100644 docs/codemods/python/pixee_python_django-model-without-dunder-str.md create mode 100644 docs/codemods/python/pixee_python_fix-dataclass-defaults.md create mode 100644 docs/codemods/python/pixee_python_fix-float-equality.md create mode 100644 docs/codemods/python/pixee_python_fix-hasattr-call.md create mode 100644 docs/codemods/python/pixee_python_fix-math-isclose.md create mode 100644 docs/codemods/python/pixee_python_fix-missing-self-or-cls.md create mode 100644 docs/codemods/python/pixee_python_harden-pickle-load.md create mode 100644 docs/codemods/python/sonar_python_break-or-continue-out-of-loop-S1716.md create mode 100644 docs/codemods/python/sonar_python_disable-graphql-introspection-S6786.md create mode 100644 docs/codemods/python/sonar_python_django-model-without-dunder-str-S6554.md create mode 100644 docs/codemods/python/sonar_python_enable-jinja2-autoescape-S5247.md create mode 100644 docs/codemods/python/sonar_python_fix-float-equality-S1244.md create mode 100644 docs/codemods/python/sonar_python_fix-math-isclose-S6727.md create mode 100644 docs/codemods/python/sonar_python_fix-missing-self-or-cls-S5719.md create mode 100644 docs/codemods/python/sonar_python_jwt-decode-verify-S5659.md create mode 100644 docs/codemods/python/sonar_python_secure-random-S2245.md create mode 100644 docs/codemods/python/sonar_python_secure-tempfile-S5445.md create mode 100644 docs/codemods/python/sonar_python_sql-parameterization-S3649.md create mode 100644 docs/codemods/python/sonar_python_url-sandbox-S5144.md diff --git a/docs/codemods/python/defectdojo_python_avoid-insecure-deserialization.md b/docs/codemods/python/defectdojo_python_avoid-insecure-deserialization.md new file mode 100644 index 0000000..ef2799b --- /dev/null +++ b/docs/codemods/python/defectdojo_python_avoid-insecure-deserialization.md @@ -0,0 +1,66 @@ +--- +title: "Harden potentially insecure deserialization operations" +sidebar_position: 1 +--- + +## defectdojo:python/avoid-insecure-deserialization + +| Importance | Review Guidance | Requires Scanning Tool | +| ---------- | -------------------------- | ---------------------- | +| High | Merge After Cursory Review | Yes (DefectDojo) | + +Use of insecure deserialization can potentially lead to arbitrary code execution. This codemod addresses this issue with two different serialization providers: + +- `yaml` (via [PyYAML](https://pyyaml.org/wiki/PyYAMLDocumentation)) +- `pickle` (via the standard library [pickle](https://docs.python.org/3/library/pickle.html) module) + +Each is described in more detail below. + +## PyYAML + +The default loaders in PyYAML are not safe to use with untrusted data. They potentially make your application vulnerable to arbitrary code execution attacks. If you open a YAML file from an untrusted source, and the file is loaded with the default loader, an attacker could execute arbitrary code on your machine. + +Calling `yaml.load()` without an explicit loader argument is equivalent to calling it with `Loader=yaml.Loader`, which is unsafe. This usage [has been deprecated](https://github.com/yaml/pyyaml/wiki/PyYAML-yaml.load(input\)-Deprecation) since PyYAML 5.1. This codemod will add an explicit `SafeLoader` argument to all `yaml.load()` calls that don't use an explicit loader. + +The changes from this codemod look like the following: + +```diff + import yaml + data = b'!!python/object/apply:subprocess.Popen \\n- ls' +- deserialized_data = yaml.load(data, yaml.Loader) ++ deserialized_data = yaml.load(data, Loader=yaml.SafeLoader) +``` + +## Pickle + +Python's `pickle` module is notoriouly insecure. While it is very useful for serializing and deserializing Python objects, it is not safe to use `pickle` to load data from untrusted sources. This is because `pickle` can execute arbitrary code when loading data. This can be exploited by an attacker to execute arbitrary code on your system. Unlike `yaml` there is no concept of a "safe" loader in `pickle`. Therefore, it is recommended to avoid `pickle` and to use a different serialization format such as `json` or `yaml` when working with untrusted data. + +However, if you must use `pickle` to load data from an untrusted source, we recommend using the open-source `fickling` library. `fickling` is a drop-in replacement for `pickle` that validates the data before loading it and checks for the possibility of code execution. This makes it much safer (although still not entirely safe) to use `pickle` to load data from untrusted sources. + +This codemod replaces calls to `pickle.load` with `fickling.load` in Python code. It also adds an import statement for `fickling` if it is not already present. + +The changes look like the following: + +```diff +- import pickle ++ import fickling + +- data = pickle.load(file) ++ data = fickling.load(file) +``` + +If you have feedback on this codemod, [please let us know](mailto:feedback@pixee.ai)! + +## F.A.Q. + +### Why is this codemod marked as Merge After Cursory Review? + +This change is generally safe and will prevent deserialization vulnerabilities. + +## Codemod Settings + +N/A + +## References + +N/A diff --git a/docs/codemods/python/defectdojo_python_django-secure-set-cookie.md b/docs/codemods/python/defectdojo_python_django-secure-set-cookie.md new file mode 100644 index 0000000..fc50aea --- /dev/null +++ b/docs/codemods/python/defectdojo_python_django-secure-set-cookie.md @@ -0,0 +1,39 @@ +--- +title: "Use Safe Parameters in Django Response `set_cookie` Call" +sidebar_position: 1 +--- + +## defectdojo:python/django-secure-set-cookie + +| Importance | Review Guidance | Requires Scanning Tool | +| ---------- | -------------------------- | ---------------------- | +| Medium | Merge After Cursory Review | Yes (DefectDojo) | + +This codemod sets the most secure parameters when Django applications call `set_cookie` on a response object. Without these parameters, your Django application cookies may be vulnerable to being intercepted and used to gain access to sensitive data. + +The changes from this codemod look like this: + +```diff + from django.shortcuts import render + def index(request): + resp = render(request, 'index.html') + - resp.set_cookie('custom_cookie', 'value') + + resp.set_cookie('custom_cookie', 'value', secure=True, httponly=True, samesite='Lax') + return resp +``` + +If you have feedback on this codemod, [please let us know](mailto:feedback@pixee.ai)! + +## F.A.Q. + +### Why is this codemod marked as Merge After Cursory Review? + +Our change provides the most secure way to create cookies in Django. However, it's possible you have configured your Django application configurations to use secure cookies. In these cases, using the default parameters for `set_cookie` is safe. + +## Codemod Settings + +N/A + +## References + +N/A diff --git a/docs/codemods/python/pixee_python_break-or-continue-out-of-loop.md b/docs/codemods/python/pixee_python_break-or-continue-out-of-loop.md new file mode 100644 index 0000000..b378f6e --- /dev/null +++ b/docs/codemods/python/pixee_python_break-or-continue-out-of-loop.md @@ -0,0 +1,36 @@ +--- +title: "Removed break or continue statement out of loop" +sidebar_position: 1 +--- + +## pixee:python/break-or-continue-out-of-loop + +| Importance | Review Guidance | Requires Scanning Tool | +| ---------- | ------------------ | ---------------------- | +| Low | Merge After Review | No | + +Any `break` or `continue` statements that are not inside a `for` or `while` loop will result in a `SyntaxError`. This codemod will remove them. + +Our changes look something like this: + +```diff +def f(): + print('not in a loop') +- break +``` + +If you have feedback on this codemod, [please let us know](mailto:feedback@pixee.ai)! + +## F.A.Q. + +### Why is this codemod marked as Merge After Review? + +While this change will make the code consistent, it is likely that the `break` or `continue` statement is a symptom of an error in program logic. + +## Codemod Settings + +N/A + +## References + +- [https://pylint.readthedocs.io/en/stable/user_guide/messages/error/not-in-loop.html](https://pylint.readthedocs.io/en/stable/user_guide/messages/error/not-in-loop.html) diff --git a/docs/codemods/python/pixee_python_combine-isinstance-issubclass.md b/docs/codemods/python/pixee_python_combine-isinstance-issubclass.md new file mode 100644 index 0000000..e74dc1d --- /dev/null +++ b/docs/codemods/python/pixee_python_combine-isinstance-issubclass.md @@ -0,0 +1,39 @@ +--- +title: "Simplify Boolean Expressions Using `isinstance` and `issubclass`" +sidebar_position: 1 +--- + +## pixee:python/combine-isinstance-issubclass + +| Importance | Review Guidance | Requires Scanning Tool | +| ---------- | -------------------- | ---------------------- | +| Low | Merge Without Review | No | + +Many developers are not necessarily aware that the `isinstance` and `issubclass` builtin methods can accept a tuple of classes to match. This means that there is a lot of code that uses boolean expressions such as `isinstance(x, str) or isinstance(x, bytes)` instead of the simpler expression `isinstance(x, (str, bytes))`. + +This codemod simplifies the boolean expressions where possible which leads to cleaner and more concise code. + +The changes from this codemod look like this: + +```diff + x = 'foo' +- if isinstance(x, str) or isinstance(x, bytes): ++ if isinstance(x, (str, bytes)): + ... +``` + +If you have feedback on this codemod, [please let us know](mailto:feedback@pixee.ai)! + +## F.A.Q. + +### Why is this codemod marked as Merge Without Review? + +Simplifying expressions involving `isinstance` or `issubclass` calls is safe. + +## Codemod Settings + +N/A + +## References + +N/A diff --git a/docs/codemods/python/pixee_python_disable-graphql-introspection.md b/docs/codemods/python/pixee_python_disable-graphql-introspection.md new file mode 100644 index 0000000..de98f85 --- /dev/null +++ b/docs/codemods/python/pixee_python_disable-graphql-introspection.md @@ -0,0 +1,61 @@ +--- +title: "Disable GraphQL Introspection to Prevent Sensitive Data Leakage" +sidebar_position: 1 +--- + +## pixee:python/disable-graphql-introspection + +| Importance | Review Guidance | Requires Scanning Tool | +| ---------- | ------------------ | ---------------------- | +| High | Merge After Review | No | + +Introspection allows a client to query the schema of a GraphQL API. Allowing introspection in production code may allow a malicious user to gather information about data types and operations for a potential attack. + +Introspection is often enabled by default in GraphQL without authentication. This codemod disables introspection altogether at the view level by introducing a validation rule. The required rules may be dependent on the framework that you are using. Please check your framework documentation for specific rules for disabling introspection. + +Our changes look something like this: + +```diff +from graphql_server.flask import GraphQLView +from flask import Flask +from graphql import ( + GraphQLSchema, GraphQLObjectType, GraphQLField, GraphQLString) ++from graphql.validation import NoSchemaIntrospectionCustomRule + +schema = GraphQLSchema( + query=GraphQLObjectType( + name='RootQueryType', + fields={ + 'hello': GraphQLField( + GraphQLString, + resolve=lambda obj, info: 'world') + })) + +app = Flask(__name__) + +app.add_url_rule("/api", + view_func=GraphQLView.as_view( # Noncompliant + name="api", + schema=schema, ++ validation_rules = [NoSchemaIntrospectionCustomRule] + ), +) +``` + +If you have feedback on this codemod, [please let us know](mailto:feedback@pixee.ai)! + +## F.A.Q. + +### Why is this codemod marked as Merge After Review? + +This change may disable a feature that was left on purpose. Moreover the rule added may be framework specific. + +## Codemod Settings + +N/A + +## References + +- [https://owasp.org/Top10/A05_2021-Security_Misconfiguration/](https://owasp.org/Top10/A05_2021-Security_Misconfiguration/) +- [https://owasp.org/www-project-top-ten/2017/A3_2017-Sensitive_Data_Exposure](https://owasp.org/www-project-top-ten/2017/A3_2017-Sensitive_Data_Exposure) +- [https://owasp.org/www-project-web-security-testing-guide/v42/4-Web_Application_Security_Testing/12-API_Testing/01-Testing_GraphQL#introspection-queries](https://owasp.org/www-project-web-security-testing-guide/v42/4-Web_Application_Security_Testing/12-API_Testing/01-Testing_GraphQL#introspection-queries) diff --git a/docs/codemods/python/pixee_python_django-debug-flag-on.md b/docs/codemods/python/pixee_python_django-debug-flag-on.md index 874d85f..f18f5c5 100644 --- a/docs/codemods/python/pixee_python_django-debug-flag-on.md +++ b/docs/codemods/python/pixee_python_django-debug-flag-on.md @@ -9,7 +9,7 @@ sidebar_position: 1 | ---------- | -------------------------- | ---------------------- | | Medium | Merge After Cursory Review | No | -This codemod will flip django's `DEBUG` flag to `False` if it's `True` on the `settings.py` file within django's default directory structure. +This codemod will flip Django's `DEBUG` flag to `False` if it's `True` on the `settings.py` file within Django's default directory structure. Having the debug flag on may result in sensitive information exposure. When an exception occurs while the `DEBUG` flag in on, it will dump metadata of your environment, including the settings module. The attacker can purposefully request a non-existing url to trigger an exception and gather information about your system. diff --git a/docs/codemods/python/pixee_python_django-model-without-dunder-str.md b/docs/codemods/python/pixee_python_django-model-without-dunder-str.md new file mode 100644 index 0000000..c65b955 --- /dev/null +++ b/docs/codemods/python/pixee_python_django-model-without-dunder-str.md @@ -0,0 +1,61 @@ +--- +title: "Ensure Django Model Classes Implement a `__str__` Method" +sidebar_position: 1 +--- + +## pixee:python/django-model-without-dunder-str + +| Importance | Review Guidance | Requires Scanning Tool | +| ---------- | ------------------ | ---------------------- | +| Low | Merge After Review | No | + +If you've ever actively developed or debugged a Django application, you may have noticed that the string representations of Django models and their instances can sometimes be hard to read or to distinguish from one another. Loading models in the interactive Django console or viewing them in the admin interface can be puzzling. This is because the default string representation of Django models is fairly generic. + +This codemod is intended to make the string representation of your model objects more human-readable. It will automatically detect all of your model's fields and display them as a descriptive string. + +For example, the default string representation of the `Question` model from Django's popular Poll App tutorial looks like this: + +```diff +from django.db import models + +class Question(models.Model): + question_text = models.CharField(max_length=200) + pub_date = models.DateTimeField("date published") ++ ++ def __str__(self): ++ model_name = self.__class__.__name__ ++ fields_str = ", ".join((f"{field.name}={getattr(self, field.name)}" for field in self._meta.fields)) ++ return f"{model_name}({fields_str})" +``` + +Without this change, the string representation of `Question` objects look like this in the interactive Django shell: + +``` +>>> Question.objects.all() +]> +``` + +With this codemod's addition of `__str__`, it now looks like: + +``` +>>> Question.objects.all() +]> +``` + +You'll notice this change works great for models with only a handful of fields. We encourage you to use this codemod's change as a starting point for further customization. + +If you have feedback on this codemod, [please let us know](mailto:feedback@pixee.ai)! + +## F.A.Q. + +### Why is this codemod marked as Merge After Review? + +This codemod is a great starting point for models with few fields. We encourage you to write custom `__str__` methods that best suit your Django application. + +## Codemod Settings + +N/A + +## References + +- [https://docs.djangoproject.com/en/5.0/ref/models/instances/#django.db.models.Model.**str**](https://docs.djangoproject.com/en/5.0/ref/models/instances/#django.db.models.Model.__str__) diff --git a/docs/codemods/python/pixee_python_django-receiver-on-top.md b/docs/codemods/python/pixee_python_django-receiver-on-top.md index 008f0e8..cd1905a 100644 --- a/docs/codemods/python/pixee_python_django-receiver-on-top.md +++ b/docs/codemods/python/pixee_python_django-receiver-on-top.md @@ -7,7 +7,7 @@ sidebar_position: 1 | Importance | Review Guidance | Requires Scanning Tool | | ---------- | -------------------- | ---------------------- | -| Medium | Merge Without Review | No | +| Low | Merge Without Review | No | Django uses signals to notify and handle actions that happens elsewhere in the application. You can define a response to a given signal by decorating a function with the `@receiver(signal)` decorator. The order in which the decorators are declared for this function is important. If the `@receiver` decorator is not on top, any decorators before it will be ignored. Our changes look something like this: diff --git a/docs/codemods/python/pixee_python_django-session-cookie-secure-off.md b/docs/codemods/python/pixee_python_django-session-cookie-secure-off.md index de41dbf..ac17f12 100644 --- a/docs/codemods/python/pixee_python_django-session-cookie-secure-off.md +++ b/docs/codemods/python/pixee_python_django-session-cookie-secure-off.md @@ -9,7 +9,7 @@ sidebar_position: 1 | ---------- | -------------------------- | ---------------------- | | Medium | Merge After Cursory Review | No | -This codemod will set django's `SESSION_COOKIE_SECURE` flag to `True` if it's `False` or missing on the `settings.py` file within django's default directory structure. +This codemod will set Django's `SESSION_COOKIE_SECURE` flag to `True` if it's `False` or missing on the `settings.py` file within Django's default directory structure. ```diff + SESSION_COOKIE_SECURE = True diff --git a/docs/codemods/python/pixee_python_fix-assert-tuple.md b/docs/codemods/python/pixee_python_fix-assert-tuple.md index a7ae52a..3d00ef8 100644 --- a/docs/codemods/python/pixee_python_fix-assert-tuple.md +++ b/docs/codemods/python/pixee_python_fix-assert-tuple.md @@ -7,7 +7,7 @@ sidebar_position: 1 | Importance | Review Guidance | Requires Scanning Tool | | ---------- | -------------------------- | ---------------------- | -| Medium | Merge After Cursory Review | No | +| Low | Merge After Cursory Review | No | An assertion on a non-empty tuple will always evaluate to `True`. This means that `assert` statements involving non-empty tuple literals are likely unintentional and should be rewritten. This codemod rewrites the original `assert` statement by creating a new `assert` for each item in the original tuple. diff --git a/docs/codemods/python/pixee_python_fix-dataclass-defaults.md b/docs/codemods/python/pixee_python_fix-dataclass-defaults.md new file mode 100644 index 0000000..665b764 --- /dev/null +++ b/docs/codemods/python/pixee_python_fix-dataclass-defaults.md @@ -0,0 +1,45 @@ +--- +title: "Replace `dataclass` Mutable Default Values with Call to `field`" +sidebar_position: 1 +--- + +## pixee:python/fix-dataclass-defaults + +| Importance | Review Guidance | Requires Scanning Tool | +| ---------- | -------------------- | ---------------------- | +| Low | Merge Without Review | No | + +When defining a Python dataclass it is not safe to use mutable datatypes (such as `list`, `dict`, or `set`) as defaults for the attributes. This is because the defined attribute will be shared by all instances of the dataclass type. Using such a mutable default will ultimately result in a `ValueError` at runtime. This codemod updates attributes of `dataclasses.dataclass` with mutable defaults to use `dataclasses.field` instead. The [dataclass documentation](https://docs.python.org/3/library/dataclasses.html#mutable-default-values) providesmore details about why using `field(default_factory=...)` is the recommended pattern. + +Our changes look something like this: + +```diff +-from dataclasses import dataclass ++from dataclasses import field, dataclass + + @dataclass + class Person: + name: str = "" +- phones: list = [] +- friends: dict = {} +- family: set = set() ++ phones: list = field(default_factory=list) ++ friends: dict = field(default_factory=dict) ++ family: set = field(default_factory=set) +``` + +If you have feedback on this codemod, [please let us know](mailto:feedback@pixee.ai)! + +## F.A.Q. + +### Why is this codemod marked as Merge Without Review? + +This change is safe and will prevent runtime `ValueError`. + +## Codemod Settings + +N/A + +## References + +- [https://docs.python.org/3/library/dataclasses.html#mutable-default-values](https://docs.python.org/3/library/dataclasses.html#mutable-default-values) diff --git a/docs/codemods/python/pixee_python_fix-deprecated-abstractproperty.md b/docs/codemods/python/pixee_python_fix-deprecated-abstractproperty.md index 7bcafc4..61c6f50 100644 --- a/docs/codemods/python/pixee_python_fix-deprecated-abstractproperty.md +++ b/docs/codemods/python/pixee_python_fix-deprecated-abstractproperty.md @@ -1,5 +1,5 @@ --- -title: "Replace deprecated abstractproperty" +title: "Replace Deprecated `abc` Decorators" sidebar_position: 1 --- @@ -9,7 +9,7 @@ sidebar_position: 1 | ---------- | -------------------- | ---------------------- | | Low | Merge Without Review | No | -The `@abstractproperty` decorator from `abc` has been [deprecated](https://docs.python.org/3/library/abc.html#abc.abstractproperty) since Python 3.3. This is because it's possible to use `@property` in combination with `@abstractmethod`. +The `@abstractproperty`, `@abstractclassmethod`, and `@abstractstaticmethod` decorators from `abc` has been [deprecated](https://docs.python.org/3/library/abc.html) since Python 3.3. This is because it's possible to use `@property`, `@classmethod`, and `@staticmethod` in combination with `@abstractmethod`. Our changes look like the following: @@ -24,6 +24,8 @@ Our changes look like the following: ... ``` +and similarly for `@abstractclassmethod` and `@abstractstaticmethod`. + If you have feedback on this codemod, [please let us know](mailto:feedback@pixee.ai)! ## F.A.Q. @@ -39,3 +41,5 @@ N/A ## References - [https://docs.python.org/3/library/abc.html#abc.abstractproperty](https://docs.python.org/3/library/abc.html#abc.abstractproperty) +- [https://docs.python.org/3/library/abc.html#abc.abstractclassmethod](https://docs.python.org/3/library/abc.html#abc.abstractclassmethod) +- [https://docs.python.org/3/library/abc.html#abc.abstractstaticmethod](https://docs.python.org/3/library/abc.html#abc.abstractstaticmethod) diff --git a/docs/codemods/python/pixee_python_fix-float-equality.md b/docs/codemods/python/pixee_python_fix-float-equality.md new file mode 100644 index 0000000..6ad0dfd --- /dev/null +++ b/docs/codemods/python/pixee_python_fix-float-equality.md @@ -0,0 +1,41 @@ +--- +title: "Use `math.isclose` Instead of Direct Equality for Floats" +sidebar_position: 1 +--- + +## pixee:python/fix-float-equality + +| Importance | Review Guidance | Requires Scanning Tool | +| ---------- | ------------------ | ---------------------- | +| Low | Merge After Review | No | + +In most programming languages, floating point arithmetic is imprecise due to the way floating point numbers are stored as binary representations. Moreover, the result of calculations with floats can vary based on when rounding happens. Using equality or inequality to compare floats or their operations will almost always be imprecise and lead to bugs. + +For these reasons, this codemod changes any operations involving equality or inequality with floats to the recommended `math.isclose` function. This codemod uses the default parameter values `rel_tol=1e-09` and `abs_tol=0.0` but makes them explicit as a starting point for you to consider depending on your calculation needs. + +Our changes look like the following: + +```diff ++import math ++ + def foo(a, b): +- return a == b - 0.1 ++ return math.isclose(a, b - 0.1, rel_tol=1e-09, abs_tol=0.0) +``` + +If you have feedback on this codemod, [please let us know](mailto:feedback@pixee.ai)! + +## F.A.Q. + +### Why is this codemod marked as Merge After Review? + +This change makes your code more accurate but in some cases it may be necessary to adjust the `abs_tol` and `rel_tol` parameter values for your particular calculations. + +## Codemod Settings + +N/A + +## References + +- [https://docs.python.org/3/tutorial/floatingpoint.html#floating-point-arithmetic-issues-and-limitations](https://docs.python.org/3/tutorial/floatingpoint.html#floating-point-arithmetic-issues-and-limitations) +- [https://docs.python.org/3/library/math.html#math.isclose](https://docs.python.org/3/library/math.html#math.isclose) diff --git a/docs/codemods/python/pixee_python_fix-hasattr-call.md b/docs/codemods/python/pixee_python_fix-hasattr-call.md new file mode 100644 index 0000000..bec742e --- /dev/null +++ b/docs/codemods/python/pixee_python_fix-hasattr-call.md @@ -0,0 +1,40 @@ +--- +title: "Use `callable` builtin to check for callables" +sidebar_position: 1 +--- + +## pixee:python/fix-hasattr-call + +| Importance | Review Guidance | Requires Scanning Tool | +| ---------- | -------------------- | ---------------------- | +| Low | Merge Without Review | No | + +This codemod fixes cases where `hasattr` is used to check if an object is a callable. You likely want to use `callable` instead. This is because using `hasattr` will return different results in some cases, such as when the class implements a `__getattr__` method. + +Our changes look something like this: + +```diff + class Test: + pass + + obj = Test() +- hasattr(obj, "__call__") ++ callable(obj) +``` + +If you have feedback on this codemod, [please let us know](mailto:feedback@pixee.ai)! + +## F.A.Q. + +### Why is this codemod marked as Merge Without Review? + +We believe this change is safe because using `callable` is a more reliable way to check if an object is a callable. + +## Codemod Settings + +N/A + +## References + +- [https://docs.python.org/3/library/functions.html#callable](https://docs.python.org/3/library/functions.html#callable) +- [https://docs.python.org/3/library/functions.html#hasattr](https://docs.python.org/3/library/functions.html#hasattr) diff --git a/docs/codemods/python/pixee_python_fix-math-isclose.md b/docs/codemods/python/pixee_python_fix-math-isclose.md new file mode 100644 index 0000000..a7bb616 --- /dev/null +++ b/docs/codemods/python/pixee_python_fix-math-isclose.md @@ -0,0 +1,40 @@ +--- +title: "Add `abs_tol` to `math.isclose` Call" +sidebar_position: 1 +--- + +## pixee:python/fix-math-isclose + +| Importance | Review Guidance | Requires Scanning Tool | +| ---------- | ------------------ | ---------------------- | +| Low | Merge After Review | No | + +The default value for the `abs_tol` argument to a `math.isclose` call is `0`. Using this default when comparing a value against `0`, such as in `math.isclose(a, 0)` is equivalent to a strict equality check to `0`, which is not the intended use of the `math.isclose` function. + +This codemod adds `abs_tol=1e-09` to any call to `math.isclose` with one of of the first arguments evaluating to `0` if `abs_tol` is not already specified. `1e-09` is a starting point for you to consider depending on your calculation needs. + +Our changes look like the following: + +```diff ++import math ++ + def foo(a): +- return math.isclose(a, 0) ++ return math.isclose(a, 0, abs_tol=1e-09) +``` + +If you have feedback on this codemod, [please let us know](mailto:feedback@pixee.ai)! + +## F.A.Q. + +### Why is this codemod marked as Merge After Review? + +This change makes your code more accurate but in some cases it may be necessary to adjust the `abs_tol` parameter value for your particular calculations. + +## Codemod Settings + +N/A + +## References + +- [https://docs.python.org/3/library/math.html#math.isclose](https://docs.python.org/3/library/math.html#math.isclose) diff --git a/docs/codemods/python/pixee_python_fix-missing-self-or-cls.md b/docs/codemods/python/pixee_python_fix-missing-self-or-cls.md new file mode 100644 index 0000000..d9dfac4 --- /dev/null +++ b/docs/codemods/python/pixee_python_fix-missing-self-or-cls.md @@ -0,0 +1,42 @@ +--- +title: "Add Missing Positional Parameter for Instance and Class Methods" +sidebar_position: 1 +--- + +## pixee:python/fix-missing-self-or-cls + +| Importance | Review Guidance | Requires Scanning Tool | +| ---------- | -------------------------- | ---------------------- | +| Low | Merge After Cursory Review | No | + +Python instance methods must be defined with `self` as the first argument. Likewise, class methods must have `cls` as the first argument. This codemod will add these arguments when the method/class method has no arguments defined. + +Our changes look something like this: + +```diff + class MyClass: +- def instance_method(): ++ def instance_method(self): + print("instance_method") + + @classmethod +- def class_method(): ++ def class_method(cls): + print("class_method") +``` + +If you have feedback on this codemod, [please let us know](mailto:feedback@pixee.ai)! + +## F.A.Q. + +### Why is this codemod marked as Merge After Cursory Review? + +This change is safe and will prevent errors when calling on these instance or class methods.. + +## Codemod Settings + +N/A + +## References + +N/A diff --git a/docs/codemods/python/pixee_python_fix-mutable-params.md b/docs/codemods/python/pixee_python_fix-mutable-params.md index 1fa4c75..c4f7777 100644 --- a/docs/codemods/python/pixee_python_fix-mutable-params.md +++ b/docs/codemods/python/pixee_python_fix-mutable-params.md @@ -7,7 +7,7 @@ sidebar_position: 1 | Importance | Review Guidance | Requires Scanning Tool | | ---------- | -------------------- | ---------------------- | -| Medium | Merge Without Review | No | +| Low | Merge Without Review | No | Using mutable values for default arguments is not a safe practice. Look at the following very simple example code: diff --git a/docs/codemods/python/pixee_python_harden-pickle-load.md b/docs/codemods/python/pixee_python_harden-pickle-load.md new file mode 100644 index 0000000..4e0d7d9 --- /dev/null +++ b/docs/codemods/python/pixee_python_harden-pickle-load.md @@ -0,0 +1,45 @@ +--- +title: "Harden `pickle.load()` against deserialization attacks" +sidebar_position: 1 +--- + +## pixee:python/harden-pickle-load + +| Importance | Review Guidance | Requires Scanning Tool | +| ---------- | -------------------------- | ---------------------- | +| High | Merge After Cursory Review | No | + +Python's `pickle` module is notoriouly insecure. While it is very useful for serializing and deserializing Python objects, it is not safe to use `pickle` to load data from untrusted sources. This is because `pickle` can execute arbitrary code when loading data. This can be exploited by an attacker to execute arbitrary code on your system. Unlike `yaml` there is no concept of a "safe" loader in `pickle`. Therefore, it is recommended to avoid `pickle` and to use a different serialization format such as `json` or `yaml` when working with untrusted data. + +However, if you must use `pickle` to load data from an untrusted source, we recommend using the open-source `fickling` library. `fickling` is a drop-in replacement for `pickle` that validates the data before loading it and checks for the possibility of code execution. This makes it much safer (although still not entirely safe) to use `pickle` to load data from untrusted sources. + +This codemod replaces calls to `pickle.load` with `fickling.load` in Python code. It also adds an import statement for `fickling` if it is not already present. + +The changes look like the following: + +```diff +- import pickle ++ import fickling + +- data = pickle.load(file) ++ data = fickling.load(file) +``` + +If you have feedback on this codemod, [please let us know](mailto:feedback@pixee.ai)! + +## F.A.Q. + +### Why is this codemod marked as Merge After Cursory Review? + +This change may impact performance in some cases, but it is recommended when handling untrusted data. + +## Codemod Settings + +N/A + +## References + +- [https://docs.python.org/3/library/pickle.html](https://docs.python.org/3/library/pickle.html) +- [https://owasp.org/www-community/vulnerabilities/Deserialization_of_untrusted_data](https://owasp.org/www-community/vulnerabilities/Deserialization_of_untrusted_data) +- [https://cheatsheetseries.owasp.org/cheatsheets/Deserialization_Cheat_Sheet.html#clear-box-review_1](https://cheatsheetseries.owasp.org/cheatsheets/Deserialization_Cheat_Sheet.html#clear-box-review_1) +- [https://github.com/trailofbits/fickling](https://github.com/trailofbits/fickling) diff --git a/docs/codemods/python/pixee_python_harden-pyyaml.md b/docs/codemods/python/pixee_python_harden-pyyaml.md index 2468073..fc7fea6 100644 --- a/docs/codemods/python/pixee_python_harden-pyyaml.md +++ b/docs/codemods/python/pixee_python_harden-pyyaml.md @@ -9,10 +9,13 @@ sidebar_position: 1 | ---------- | -------------------- | ---------------------- | | Medium | Merge Without Review | No | -This codemod hardens all [`yaml.load()`](https://pyyaml.org/wiki/PyYAMLDocumentation) calls against attacks that could result from deserializing untrusted data. +The default loaders in PyYAML are not safe to use with untrusted data. They potentially make your application vulnerable to arbitrary code execution attacks. If you open a YAML file from an untrusted source, and the file is loaded with the default loader, an attacker could execute arbitrary code on your machine. -The fix uses a safety check that already exists in the `yaml` module, replacing unsafe loader class with `SafeLoader`. -The changes from this codemod look like this: +This codemod hardens all [`yaml.load()`](https://pyyaml.org/wiki/PyYAMLDocumentation) calls against such attacks by replacing the default loader with `yaml.SafeLoader`. This is the recommended loader for loading untrusted data. For most use cases it functions as a drop-in replacement for the default loader. + +Calling `yaml.load()` without an explicit loader argument is equivalent to calling it with `Loader=yaml.Loader`, which is unsafe. This usage [has been deprecated](https://github.com/yaml/pyyaml/wiki/PyYAML-yaml.load(input\)-Deprecation) since PyYAML 5.1. This codemod will add an explicit `SafeLoader` argument to all `yaml.load()` calls that don't use an explicit loader. + +The changes from this codemod look like the following: ```diff import yaml @@ -21,9 +24,6 @@ The changes from this codemod look like this: + deserialized_data = yaml.load(data, Loader=yaml.SafeLoader) ``` -The codemod will also catch if you pass in the loader argument as a kwarg and if you use any loader other than `SafeLoader`, -including `FullLoader` and `UnsafeLoader`. - If you have feedback on this codemod, [please let us know](mailto:feedback@pixee.ai)! ## F.A.Q. @@ -39,3 +39,4 @@ N/A ## References - [https://owasp.org/www-community/vulnerabilities/Deserialization_of_untrusted_data](https://owasp.org/www-community/vulnerabilities/Deserialization_of_untrusted_data) +- [https://github.com/yaml/pyyaml/wiki/PyYAML-yaml.load(input)-Deprecation]() diff --git a/docs/codemods/python/pixee_python_lazy-logging.md b/docs/codemods/python/pixee_python_lazy-logging.md index ce42b3d..6f02e51 100644 --- a/docs/codemods/python/pixee_python_lazy-logging.md +++ b/docs/codemods/python/pixee_python_lazy-logging.md @@ -7,7 +7,7 @@ sidebar_position: 1 | Importance | Review Guidance | Requires Scanning Tool | | ---------- | -------------------- | ---------------------- | -| Medium | Merge Without Review | No | +| Low | Merge Without Review | No | This codemod converts "eager" logging into "lazy" logging, which is preferred for performance efficiency and resource optimization. Lazy logging defers the actual construction and formatting of log messages until it's confirmed that the message will be logged based on the current log level, thereby avoiding unnecessary computation for messages that will not be logged. diff --git a/docs/codemods/python/pixee_python_numpy-nan-equality.md b/docs/codemods/python/pixee_python_numpy-nan-equality.md index 495112c..dc716ac 100644 --- a/docs/codemods/python/pixee_python_numpy-nan-equality.md +++ b/docs/codemods/python/pixee_python_numpy-nan-equality.md @@ -7,7 +7,7 @@ sidebar_position: 1 | Importance | Review Guidance | Requires Scanning Tool | | ---------- | -------------------- | ---------------------- | -| Medium | Merge Without Review | No | +| Low | Merge Without Review | No | Comparisons against `numpy.nan` always result in `False`. Thus comparing an expression directly against `numpy.nan` is always unintended. The correct way to compare a value for `NaN` is to use the `numpy.isnan` function. diff --git a/docs/codemods/python/pixee_python_remove-debug-breakpoint.md b/docs/codemods/python/pixee_python_remove-debug-breakpoint.md index 3b7511c..4238bc8 100644 --- a/docs/codemods/python/pixee_python_remove-debug-breakpoint.md +++ b/docs/codemods/python/pixee_python_remove-debug-breakpoint.md @@ -7,12 +7,14 @@ sidebar_position: 1 | Importance | Review Guidance | Requires Scanning Tool | | ---------- | -------------------- | ---------------------- | -| Medium | Merge Without Review | No | +| Low | Merge Without Review | No | This codemod removes any calls to `breakpoint()` or `pdb.set_trace()` which are generally only used for interactive debugging and should not be deployed in production code. In most cases if these calls are included in committed code, they were left there by mistake and indicate a potential problem. +Our changes look something like this: + ```diff print("hello") - breakpoint() diff --git a/docs/codemods/python/pixee_python_str-concat-in-sequence-literals.md b/docs/codemods/python/pixee_python_str-concat-in-sequence-literals.md index 63b8e31..8263065 100644 --- a/docs/codemods/python/pixee_python_str-concat-in-sequence-literals.md +++ b/docs/codemods/python/pixee_python_str-concat-in-sequence-literals.md @@ -7,7 +7,7 @@ sidebar_position: 1 | Importance | Review Guidance | Requires Scanning Tool | | ---------- | -------------------------- | ---------------------- | -| Medium | Merge After Cursory Review | No | +| Low | Merge After Cursory Review | No | This codemod fixes cases of implicit string concatenation inside lists, sets, or tuples. This is most likely a mistake: you probably meant include a comma in between the concatenated strings. diff --git a/docs/codemods/python/sonar_python_break-or-continue-out-of-loop-S1716.md b/docs/codemods/python/sonar_python_break-or-continue-out-of-loop-S1716.md new file mode 100644 index 0000000..d21b530 --- /dev/null +++ b/docs/codemods/python/sonar_python_break-or-continue-out-of-loop-S1716.md @@ -0,0 +1,39 @@ +--- +title: "Sonar: Removed break or continue statement out of loop" +sidebar_position: 1 +--- + +## sonar:python/break-or-continue-out-of-loop-S1716 + +| Importance | Review Guidance | Requires Scanning Tool | +| ---------- | ------------------ | ---------------------- | +| Low | Merge After Review | Yes (Sonar) | + +This codemod acts upon the following Sonar rules: python:S1716. + +Any `break` or `continue` statements that are not inside a `for` or `while` loop will result in a `SyntaxError`. This codemod will remove them. + +Our changes look something like this: + +```diff +def f(): + print('not in a loop') +- break +``` + +If you have feedback on this codemod, [please let us know](mailto:feedback@pixee.ai)! + +## F.A.Q. + +### Why is this codemod marked as Merge After Review? + +While this change will make the code consistent, it is likely that the `break` or `continue` statement is a symptom of an error in program logic. + +## Codemod Settings + +N/A + +## References + +- [https://pylint.readthedocs.io/en/stable/user_guide/messages/error/not-in-loop.html](https://pylint.readthedocs.io/en/stable/user_guide/messages/error/not-in-loop.html) +- ["break" and "continue" should not be used outside a loop](https://rules.sonarsource.com/python/RSPEC-1716/) diff --git a/docs/codemods/python/sonar_python_disable-graphql-introspection-S6786.md b/docs/codemods/python/sonar_python_disable-graphql-introspection-S6786.md new file mode 100644 index 0000000..dd8be69 --- /dev/null +++ b/docs/codemods/python/sonar_python_disable-graphql-introspection-S6786.md @@ -0,0 +1,64 @@ +--- +title: "Sonar: Disable GraphQL Introspection to Prevent Sensitive Data Leakage" +sidebar_position: 1 +--- + +## sonar:python/disable-graphql-introspection-S6786 + +| Importance | Review Guidance | Requires Scanning Tool | +| ---------- | ------------------ | ---------------------- | +| High | Merge After Review | Yes (Sonar) | + +This codemod acts upon the following Sonar rules: python:S6786. + +Introspection allows a client to query the schema of a GraphQL API. Allowing introspection in production code may allow a malicious user to gather information about data types and operations for a potential attack. + +Introspection is often enabled by default in GraphQL without authentication. This codemod disables introspection altogether at the view level by introducing a validation rule. The required rules may be dependent on the framework that you are using. Please check your framework documentation for specific rules for disabling introspection. + +Our changes look something like this: + +```diff +from graphql_server.flask import GraphQLView +from flask import Flask +from graphql import ( + GraphQLSchema, GraphQLObjectType, GraphQLField, GraphQLString) ++from graphql.validation import NoSchemaIntrospectionCustomRule + +schema = GraphQLSchema( + query=GraphQLObjectType( + name='RootQueryType', + fields={ + 'hello': GraphQLField( + GraphQLString, + resolve=lambda obj, info: 'world') + })) + +app = Flask(__name__) + +app.add_url_rule("/api", + view_func=GraphQLView.as_view( # Noncompliant + name="api", + schema=schema, ++ validation_rules = [NoSchemaIntrospectionCustomRule] + ), +) +``` + +If you have feedback on this codemod, [please let us know](mailto:feedback@pixee.ai)! + +## F.A.Q. + +### Why is this codemod marked as Merge After Review? + +This change may disable a feature that was left on purpose. Moreover the rule added may be framework specific. + +## Codemod Settings + +N/A + +## References + +- [https://owasp.org/Top10/A05_2021-Security_Misconfiguration/](https://owasp.org/Top10/A05_2021-Security_Misconfiguration/) +- [https://owasp.org/www-project-top-ten/2017/A3_2017-Sensitive_Data_Exposure](https://owasp.org/www-project-top-ten/2017/A3_2017-Sensitive_Data_Exposure) +- [https://owasp.org/www-project-web-security-testing-guide/v42/4-Web_Application_Security_Testing/12-API_Testing/01-Testing_GraphQL#introspection-queries](https://owasp.org/www-project-web-security-testing-guide/v42/4-Web_Application_Security_Testing/12-API_Testing/01-Testing_GraphQL#introspection-queries) +- [GraphQL introspection should be disabled in production](https://rules.sonarsource.com/python/RSPEC-6786/) diff --git a/docs/codemods/python/sonar_python_django-json-response-type-S5131.md b/docs/codemods/python/sonar_python_django-json-response-type-S5131.md index b8d972b..9e99640 100644 --- a/docs/codemods/python/sonar_python_django-json-response-type-S5131.md +++ b/docs/codemods/python/sonar_python_django-json-response-type-S5131.md @@ -9,7 +9,7 @@ sidebar_position: 1 | ---------- | -------------------- | ---------------------- | | Medium | Merge Without Review | Yes (Sonar) | -This codemod acts upon the following Sonar rules: 'pythonsecurity:S5131'. +This codemod acts upon the following Sonar rules: pythonsecurity:S5131. The default `content_type` for `HttpResponse` in Django is `'text/html'`. This is true even when the response contains JSON data. If the JSON contains (unsanitized) user-supplied input, a malicious user may supply HTML code which leaves the application vulnerable to cross-site scripting (XSS). @@ -41,4 +41,4 @@ N/A - [https://docs.djangoproject.com/en/4.0/ref/request-response/#django.http.HttpResponse.**init**](https://docs.djangoproject.com/en/4.0/ref/request-response/#django.http.HttpResponse.__init__) - [https://cheatsheetseries.owasp.org/cheatsheets/Cross_Site_Scripting_Prevention_Cheat_Sheet.html#output-encoding-for-javascript-contexts](https://cheatsheetseries.owasp.org/cheatsheets/Cross_Site_Scripting_Prevention_Cheat_Sheet.html#output-encoding-for-javascript-contexts) -- [https://rules.sonarsource.com/python/type/Bug/RSPEC-5131/](https://rules.sonarsource.com/python/type/Bug/RSPEC-5131/) +- [Endpoints should not be vulnerable to reflected XSS attacks (Django)](https://rules.sonarsource.com/python/type/Bug/RSPEC-5131/) diff --git a/docs/codemods/python/sonar_python_django-model-without-dunder-str-S6554.md b/docs/codemods/python/sonar_python_django-model-without-dunder-str-S6554.md new file mode 100644 index 0000000..e042a30 --- /dev/null +++ b/docs/codemods/python/sonar_python_django-model-without-dunder-str-S6554.md @@ -0,0 +1,64 @@ +--- +title: "Sonar: Ensure Django Model Classes Implement a `__str__` Method" +sidebar_position: 1 +--- + +## sonar:python/django-model-without-dunder-str-S6554 + +| Importance | Review Guidance | Requires Scanning Tool | +| ---------- | ------------------ | ---------------------- | +| Low | Merge After Review | Yes (Sonar) | + +This codemod acts upon the following Sonar rules: python:S6554. + +If you've ever actively developed or debugged a Django application, you may have noticed that the string representations of Django models and their instances can sometimes be hard to read or to distinguish from one another. Loading models in the interactive Django console or viewing them in the admin interface can be puzzling. This is because the default string representation of Django models is fairly generic. + +This codemod is intended to make the string representation of your model objects more human-readable. It will automatically detect all of your model's fields and display them as a descriptive string. + +For example, the default string representation of the `Question` model from Django's popular Poll App tutorial looks like this: + +```diff +from django.db import models + +class Question(models.Model): + question_text = models.CharField(max_length=200) + pub_date = models.DateTimeField("date published") ++ ++ def __str__(self): ++ model_name = self.__class__.__name__ ++ fields_str = ", ".join((f"{field.name}={getattr(self, field.name)}" for field in self._meta.fields)) ++ return f"{model_name}({fields_str})" +``` + +Without this change, the string representation of `Question` objects look like this in the interactive Django shell: + +``` +>>> Question.objects.all() +]> +``` + +With this codemod's addition of `__str__`, it now looks like: + +``` +>>> Question.objects.all() +]> +``` + +You'll notice this change works great for models with only a handful of fields. We encourage you to use this codemod's change as a starting point for further customization. + +If you have feedback on this codemod, [please let us know](mailto:feedback@pixee.ai)! + +## F.A.Q. + +### Why is this codemod marked as Merge After Review? + +This codemod is a great starting point for models with few fields. We encourage you to write custom `__str__` methods that best suit your Django application. + +## Codemod Settings + +N/A + +## References + +- [https://docs.djangoproject.com/en/5.0/ref/models/instances/#django.db.models.Model.**str**](https://docs.djangoproject.com/en/5.0/ref/models/instances/#django.db.models.Model.__str__) +- [Django models should define a "**str**" method](https://rules.sonarsource.com/python/RSPEC-6554/) diff --git a/docs/codemods/python/sonar_python_django-receiver-on-top-S6552.md b/docs/codemods/python/sonar_python_django-receiver-on-top-S6552.md index e75c90e..18bad62 100644 --- a/docs/codemods/python/sonar_python_django-receiver-on-top-S6552.md +++ b/docs/codemods/python/sonar_python_django-receiver-on-top-S6552.md @@ -7,9 +7,9 @@ sidebar_position: 1 | Importance | Review Guidance | Requires Scanning Tool | | ---------- | -------------------- | ---------------------- | -| Medium | Merge Without Review | Yes (Sonar) | +| Low | Merge Without Review | Yes (Sonar) | -This codemod acts upon the following Sonar rules: 'python:S6552'. +This codemod acts upon the following Sonar rules: python:S6552. Django uses signals to notify and handle actions that happens elsewhere in the application. You can define a response to a given signal by decorating a function with the `@receiver(signal)` decorator. The order in which the decorators are declared for this function is important. If the `@receiver` decorator is not on top, any decorators before it will be ignored. Our changes look something like this: @@ -41,4 +41,4 @@ N/A ## References - [https://docs.djangoproject.com/en/4.1/topics/signals/](https://docs.djangoproject.com/en/4.1/topics/signals/) -- [https://rules.sonarsource.com/python/type/Bug/RSPEC-6552/](https://rules.sonarsource.com/python/type/Bug/RSPEC-6552/) +- [Django signal handler functions should have the `@receiver` decorator on top of all other decorators](https://rules.sonarsource.com/python/type/Bug/RSPEC-6552/) diff --git a/docs/codemods/python/sonar_python_enable-jinja2-autoescape-S5247.md b/docs/codemods/python/sonar_python_enable-jinja2-autoescape-S5247.md new file mode 100644 index 0000000..844a7ea --- /dev/null +++ b/docs/codemods/python/sonar_python_enable-jinja2-autoescape-S5247.md @@ -0,0 +1,44 @@ +--- +title: "Sonar: Enable Jinja2 Autoescape" +sidebar_position: 1 +--- + +## sonar:python/enable-jinja2-autoescape-S5247 + +| Importance | Review Guidance | Requires Scanning Tool | +| ---------- | ------------------ | ---------------------- | +| High | Merge After Review | Yes (Sonar) | + +This codemod acts upon the following Sonar rules: python:S5247. + +This codemod enables autoescaping of HTML content in `jinja2`. Unfortunately, the jinja2 default behavior is to not autoescape when rendering templates, which makes your applications potentially vulnerable to Cross-Site Scripting (XSS) attacks. + +Our codemod checks if you forgot to enable autoescape or if you explicitly disabled it. The change looks as follows: + +```diff + from jinja2 import Environment + +- env = Environment() +- env = Environment(autoescape=False, loader=some_loader) ++ env = Environment(autoescape=True) ++ env = Environment(autoescape=True, loader=some_loader) + ... +``` + +If you have feedback on this codemod, [please let us know](mailto:feedback@pixee.ai)! + +## F.A.Q. + +### Why is this codemod marked as Merge After Review? + +This codemod protects your applications against XSS attacks. However, it's possible you would like to set the `autoescape` parameter to a custom callable. + +## Codemod Settings + +N/A + +## References + +- [https://owasp.org/www-community/attacks/xss/](https://owasp.org/www-community/attacks/xss/) +- [https://jinja.palletsprojects.com/en/3.1.x/api/#autoescaping](https://jinja.palletsprojects.com/en/3.1.x/api/#autoescaping) +- [Disabling auto-escaping in template engines is security-sensitive](https://rules.sonarsource.com/python/RSPEC-5247/) diff --git a/docs/codemods/python/sonar_python_exception-without-raise-S3984.md b/docs/codemods/python/sonar_python_exception-without-raise-S3984.md index a2f7c8f..83f5abe 100644 --- a/docs/codemods/python/sonar_python_exception-without-raise-S3984.md +++ b/docs/codemods/python/sonar_python_exception-without-raise-S3984.md @@ -9,7 +9,7 @@ sidebar_position: 1 | ---------- | -------------------- | ---------------------- | | Low | Merge Without Review | Yes (Sonar) | -This codemod acts upon the following Sonar rules: 'python:S3984'. +This codemod acts upon the following Sonar rules: python:S3984. This codemod fixes cases where an exception is referenced by itself in a statement without being raised. This most likely indicates a bug: you probably meant to actually raise the exception. @@ -38,4 +38,4 @@ N/A ## References - [https://docs.python.org/3/tutorial/errors.html#raising-exceptions](https://docs.python.org/3/tutorial/errors.html#raising-exceptions) -- [https://rules.sonarsource.com/python/type/Bug/RSPEC-3984/](https://rules.sonarsource.com/python/type/Bug/RSPEC-3984/) +- [Exceptions should not be created without being raised](https://rules.sonarsource.com/python/type/Bug/RSPEC-3984/) diff --git a/docs/codemods/python/sonar_python_fix-assert-tuple-S5905.md b/docs/codemods/python/sonar_python_fix-assert-tuple-S5905.md index 470c5b5..ab4aa61 100644 --- a/docs/codemods/python/sonar_python_fix-assert-tuple-S5905.md +++ b/docs/codemods/python/sonar_python_fix-assert-tuple-S5905.md @@ -7,9 +7,9 @@ sidebar_position: 1 | Importance | Review Guidance | Requires Scanning Tool | | ---------- | -------------------------- | ---------------------- | -| Medium | Merge After Cursory Review | Yes (Sonar) | +| Low | Merge After Cursory Review | Yes (Sonar) | -This codemod acts upon the following Sonar rules: 'python:S5905'. +This codemod acts upon the following Sonar rules: python:S5905. An assertion on a non-empty tuple will always evaluate to `True`. This means that `assert` statements involving non-empty tuple literals are likely unintentional and should be rewritten. This codemod rewrites the original `assert` statement by creating a new `assert` for each item in the original tuple. @@ -35,4 +35,4 @@ N/A ## References -- [https://rules.sonarsource.com/python/type/Bug/RSPEC-5905/](https://rules.sonarsource.com/python/type/Bug/RSPEC-5905/) +- [Assert should not be called on a tuple literal](https://rules.sonarsource.com/python/type/Bug/RSPEC-5905/) diff --git a/docs/codemods/python/sonar_python_fix-float-equality-S1244.md b/docs/codemods/python/sonar_python_fix-float-equality-S1244.md new file mode 100644 index 0000000..750d40b --- /dev/null +++ b/docs/codemods/python/sonar_python_fix-float-equality-S1244.md @@ -0,0 +1,44 @@ +--- +title: "Sonar: Use `math.isclose` Instead of Direct Equality for Floats" +sidebar_position: 1 +--- + +## sonar:python/fix-float-equality-S1244 + +| Importance | Review Guidance | Requires Scanning Tool | +| ---------- | ------------------ | ---------------------- | +| Low | Merge After Review | Yes (Sonar) | + +This codemod acts upon the following Sonar rules: python:S1244. + +In most programming languages, floating point arithmetic is imprecise due to the way floating point numbers are stored as binary representations. Moreover, the result of calculations with floats can vary based on when rounding happens. Using equality or inequality to compare floats or their operations will almost always be imprecise and lead to bugs. + +For these reasons, this codemod changes any operations involving equality or inequality with floats to the recommended `math.isclose` function. This codemod uses the default parameter values `rel_tol=1e-09` and `abs_tol=0.0` but makes them explicit as a starting point for you to consider depending on your calculation needs. + +Our changes look like the following: + +```diff ++import math ++ + def foo(a, b): +- return a == b - 0.1 ++ return math.isclose(a, b - 0.1, rel_tol=1e-09, abs_tol=0.0) +``` + +If you have feedback on this codemod, [please let us know](mailto:feedback@pixee.ai)! + +## F.A.Q. + +### Why is this codemod marked as Merge After Review? + +This change makes your code more accurate but in some cases it may be necessary to adjust the `abs_tol` and `rel_tol` parameter values for your particular calculations. + +## Codemod Settings + +N/A + +## References + +- [https://docs.python.org/3/tutorial/floatingpoint.html#floating-point-arithmetic-issues-and-limitations](https://docs.python.org/3/tutorial/floatingpoint.html#floating-point-arithmetic-issues-and-limitations) +- [https://docs.python.org/3/library/math.html#math.isclose](https://docs.python.org/3/library/math.html#math.isclose) +- [Floating point numbers should not be tested for equality](https://rules.sonarsource.com/python/type/Bug/RSPEC-1244/) diff --git a/docs/codemods/python/sonar_python_fix-math-isclose-S6727.md b/docs/codemods/python/sonar_python_fix-math-isclose-S6727.md new file mode 100644 index 0000000..5a467c0 --- /dev/null +++ b/docs/codemods/python/sonar_python_fix-math-isclose-S6727.md @@ -0,0 +1,43 @@ +--- +title: "Sonar: Add `abs_tol` to `math.isclose` Call" +sidebar_position: 1 +--- + +## sonar:python/fix-math-isclose-S6727 + +| Importance | Review Guidance | Requires Scanning Tool | +| ---------- | ------------------ | ---------------------- | +| Low | Merge After Review | Yes (Sonar) | + +This codemod acts upon the following Sonar rules: python:S6727. + +The default value for the `abs_tol` argument to a `math.isclose` call is `0`. Using this default when comparing a value against `0`, such as in `math.isclose(a, 0)` is equivalent to a strict equality check to `0`, which is not the intended use of the `math.isclose` function. + +This codemod adds `abs_tol=1e-09` to any call to `math.isclose` with one of of the first arguments evaluating to `0` if `abs_tol` is not already specified. `1e-09` is a starting point for you to consider depending on your calculation needs. + +Our changes look like the following: + +```diff ++import math ++ + def foo(a): +- return math.isclose(a, 0) ++ return math.isclose(a, 0, abs_tol=1e-09) +``` + +If you have feedback on this codemod, [please let us know](mailto:feedback@pixee.ai)! + +## F.A.Q. + +### Why is this codemod marked as Merge After Review? + +This change makes your code more accurate but in some cases it may be necessary to adjust the `abs_tol` parameter value for your particular calculations. + +## Codemod Settings + +N/A + +## References + +- [https://docs.python.org/3/library/math.html#math.isclose](https://docs.python.org/3/library/math.html#math.isclose) +- [The abs_tol parameter should be provided when using math.isclose to compare values to 0](https://rules.sonarsource.com/python/RSPEC-6727/) diff --git a/docs/codemods/python/sonar_python_fix-missing-self-or-cls-S5719.md b/docs/codemods/python/sonar_python_fix-missing-self-or-cls-S5719.md new file mode 100644 index 0000000..c35b211 --- /dev/null +++ b/docs/codemods/python/sonar_python_fix-missing-self-or-cls-S5719.md @@ -0,0 +1,44 @@ +--- +title: "Sonar: Add Missing Positional Parameter for Instance and Class Methods" +sidebar_position: 1 +--- + +## sonar:python/fix-missing-self-or-cls-S5719 + +| Importance | Review Guidance | Requires Scanning Tool | +| ---------- | -------------------------- | ---------------------- | +| Low | Merge After Cursory Review | Yes (Sonar) | + +This codemod acts upon the following Sonar rules: python:S5719. + +Python instance methods must be defined with `self` as the first argument. Likewise, class methods must have `cls` as the first argument. This codemod will add these arguments when the method/class method has no arguments defined. + +Our changes look something like this: + +```diff + class MyClass: +- def instance_method(): ++ def instance_method(self): + print("instance_method") + + @classmethod +- def class_method(): ++ def class_method(cls): + print("class_method") +``` + +If you have feedback on this codemod, [please let us know](mailto:feedback@pixee.ai)! + +## F.A.Q. + +### Why is this codemod marked as Merge After Cursory Review? + +This change is safe and will prevent errors when calling on these instance or class methods.. + +## Codemod Settings + +N/A + +## References + +- [Instance and class methods should have at least one positional parameter](https://rules.sonarsource.com/python/RSPEC-5719/) diff --git a/docs/codemods/python/sonar_python_flask-json-response-type-S5131.md b/docs/codemods/python/sonar_python_flask-json-response-type-S5131.md index eb8a13b..a10dd8e 100644 --- a/docs/codemods/python/sonar_python_flask-json-response-type-S5131.md +++ b/docs/codemods/python/sonar_python_flask-json-response-type-S5131.md @@ -9,7 +9,7 @@ sidebar_position: 1 | ---------- | -------------------- | ---------------------- | | Medium | Merge Without Review | Yes (Sonar) | -This codemod acts upon the following Sonar rules: 'pythonsecurity:S5131'. +This codemod acts upon the following Sonar rules: pythonsecurity:S5131. The default `mimetype` for `make_response` in Flask is `'text/html'`. This is true even when the response contains JSON data. If the JSON contains (unsanitized) user-supplied input, a malicious user may supply HTML code which leaves the application vulnerable to cross-site scripting (XSS). @@ -44,4 +44,4 @@ N/A - [https://flask.palletsprojects.com/en/2.3.x/patterns/javascript/#return-json-from-views](https://flask.palletsprojects.com/en/2.3.x/patterns/javascript/#return-json-from-views) - [https://cheatsheetseries.owasp.org/cheatsheets/Cross_Site_Scripting_Prevention_Cheat_Sheet.html#output-encoding-for-javascript-contexts](https://cheatsheetseries.owasp.org/cheatsheets/Cross_Site_Scripting_Prevention_Cheat_Sheet.html#output-encoding-for-javascript-contexts) -- [https://rules.sonarsource.com/python/type/Bug/RSPEC-5131/](https://rules.sonarsource.com/python/type/Bug/RSPEC-5131/) +- [Endpoints should not be vulnerable to reflected XSS attacks (Flask)](https://rules.sonarsource.com/python/type/Bug/RSPEC-5131/) diff --git a/docs/codemods/python/sonar_python_jwt-decode-verify-S5659.md b/docs/codemods/python/sonar_python_jwt-decode-verify-S5659.md new file mode 100644 index 0000000..2cd342e --- /dev/null +++ b/docs/codemods/python/sonar_python_jwt-decode-verify-S5659.md @@ -0,0 +1,46 @@ +--- +title: "Sonar: Verify JWT Decode" +sidebar_position: 1 +--- + +## sonar:python/jwt-decode-verify-S5659 + +| Importance | Review Guidance | Requires Scanning Tool | +| ---------- | -------------------- | ---------------------- | +| High | Merge Without Review | Yes (Sonar) | + +This codemod acts upon the following Sonar rules: python:S5659. + +This codemod ensures calls to [jwt.decode](https://pyjwt.readthedocs.io/en/stable/api.html#jwt.decode) do not disable signature validation and other verifications. It checks that both the `verify` parameter (soon to be deprecated) and any `verify` key in the `options` dict parameter are not assigned to `False`. + +Our change looks as follows: + +```diff + import jwt + ... +- decoded_payload = jwt.decode(encoded_jwt, SECRET_KEY, algorithms=["HS256"], verify=False) ++ decoded_payload = jwt.decode(encoded_jwt, SECRET_KEY, algorithms=["HS256"], verify=True) + ... +- decoded_payload = jwt.decode(encoded_jwt, SECRET_KEY, algorithms=["HS256"], options={"verify_signature": False, "verify_exp": False}) ++ decoded_payload = jwt.decode(encoded_jwt, SECRET_KEY, algorithms=["HS256"], options={"verify_signature": True, "verify_exp": True}) +``` + +Any `verify` parameter not listed relies on the secure `True` default value. + +If you have feedback on this codemod, [please let us know](mailto:feedback@pixee.ai)! + +## F.A.Q. + +### Why is this codemod marked as Merge Without Review? + +This codemod ensures your code uses all available validations when calling `jwt.decode`. We believe this replacement is safe and should not result in any issues. + +## Codemod Settings + +N/A + +## References + +- [https://pyjwt.readthedocs.io/en/stable/api.html](https://pyjwt.readthedocs.io/en/stable/api.html) +- [https://owasp.org/www-project-web-security-testing-guide/latest/4-Web_Application_Security_Testing/06-Session_Management_Testing/10-Testing_JSON_Web_Tokens](https://owasp.org/www-project-web-security-testing-guide/latest/4-Web_Application_Security_Testing/06-Session_Management_Testing/10-Testing_JSON_Web_Tokens) +- [JWT should be signed and verified](https://rules.sonarsource.com/python/RSPEC-5659/) diff --git a/docs/codemods/python/sonar_python_literal-or-new-object-identity-S5796.md b/docs/codemods/python/sonar_python_literal-or-new-object-identity-S5796.md index e4d85d0..2ce9b8c 100644 --- a/docs/codemods/python/sonar_python_literal-or-new-object-identity-S5796.md +++ b/docs/codemods/python/sonar_python_literal-or-new-object-identity-S5796.md @@ -9,7 +9,7 @@ sidebar_position: 1 | ---------- | -------------------- | ---------------------- | | Low | Merge Without Review | Yes (Sonar) | -This codemod acts upon the following Sonar rules: 'python:S5796'. +This codemod acts upon the following Sonar rules: python:S5796. The `is` and `is not` operators only evaluate to `True` when the expressions on each side have the same `id`. In other words, `a is b` is equivalent to `id(a) == id(b)`. With few exceptions, objects and literals have unique identities and thus shouldn't generally be compared by using the `is` or `is not` operators. @@ -36,4 +36,4 @@ N/A ## References - [https://docs.python.org/3/library/stdtypes.html#comparisons](https://docs.python.org/3/library/stdtypes.html#comparisons) -- [https://rules.sonarsource.com/python/type/Bug/RSPEC-5796/](https://rules.sonarsource.com/python/type/Bug/RSPEC-5796/) +- [New objects should not be created only to check their identity](https://rules.sonarsource.com/python/type/Bug/RSPEC-5796/) diff --git a/docs/codemods/python/sonar_python_numpy-nan-equality-S6725.md b/docs/codemods/python/sonar_python_numpy-nan-equality-S6725.md index c2fc591..eba0977 100644 --- a/docs/codemods/python/sonar_python_numpy-nan-equality-S6725.md +++ b/docs/codemods/python/sonar_python_numpy-nan-equality-S6725.md @@ -7,9 +7,9 @@ sidebar_position: 1 | Importance | Review Guidance | Requires Scanning Tool | | ---------- | -------------------- | ---------------------- | -| Medium | Merge Without Review | Yes (Sonar) | +| Low | Merge Without Review | Yes (Sonar) | -This codemod acts upon the following Sonar rules: 'python:S6725'. +This codemod acts upon the following Sonar rules: python:S6725. Comparisons against `numpy.nan` always result in `False`. Thus comparing an expression directly against `numpy.nan` is always unintended. The correct way to compare a value for `NaN` is to use the `numpy.isnan` function. @@ -39,4 +39,4 @@ N/A ## References - [https://numpy.org/doc/stable/reference/constants.html#numpy.nan](https://numpy.org/doc/stable/reference/constants.html#numpy.nan) -- [https://rules.sonarsource.com/python/type/Bug/RSPEC-6725/](https://rules.sonarsource.com/python/type/Bug/RSPEC-6725/) +- [Equality checks should not be made against `numpy.nan`](https://rules.sonarsource.com/python/type/Bug/RSPEC-6725/) diff --git a/docs/codemods/python/sonar_python_remove-assertion-in-pytest-raises-S5915.md b/docs/codemods/python/sonar_python_remove-assertion-in-pytest-raises-S5915.md index 94c2e52..a54dc1c 100644 --- a/docs/codemods/python/sonar_python_remove-assertion-in-pytest-raises-S5915.md +++ b/docs/codemods/python/sonar_python_remove-assertion-in-pytest-raises-S5915.md @@ -9,7 +9,7 @@ sidebar_position: 1 | ---------- | -------------------- | ---------------------- | | Low | Merge Without Review | Yes (Sonar) | -This codemod acts upon the following Sonar rules: 'python:S5915'. +This codemod acts upon the following Sonar rules: python:S5915. The context manager object `pytest.raises()` will assert if the code contained within its scope will raise an exception of type ``. The documentation points that the exception must be raised in the last line of its scope and any line afterwards won't be executed. Including asserts at the end of the scope is a common error. This codemod addresses that by moving them out of the scope. @@ -42,4 +42,4 @@ N/A ## References - [https://docs.pytest.org/en/7.4.x/reference/reference.html#pytest-raises](https://docs.pytest.org/en/7.4.x/reference/reference.html#pytest-raises) -- [https://rules.sonarsource.com/python/type/Bug/RSPEC-5915/](https://rules.sonarsource.com/python/type/Bug/RSPEC-5915/) +- [Assertions should not be made at the end of blocks expecting an exception](https://rules.sonarsource.com/python/type/Bug/RSPEC-5915/) diff --git a/docs/codemods/python/sonar_python_secure-random-S2245.md b/docs/codemods/python/sonar_python_secure-random-S2245.md new file mode 100644 index 0000000..2fffa79 --- /dev/null +++ b/docs/codemods/python/sonar_python_secure-random-S2245.md @@ -0,0 +1,46 @@ +--- +title: "Sonar: Secure Source of Randomness" +sidebar_position: 1 +--- + +## sonar:python/secure-random-S2245 + +| Importance | Review Guidance | Requires Scanning Tool | +| ---------- | -------------------------- | ---------------------- | +| High | Merge After Cursory Review | Yes (Sonar) | + +This codemod acts upon the following Sonar rules: python:S2245. + +This codemod replaces all instances of functions in the `random` module (e.g. `random.random()` with their, much more secure, equivalents from the `secrets` module (e.g. `secrets.SystemRandom().random()`). + +There is significant algorithmic complexity in getting computers to generate genuinely unguessable random bits. The `random.random()` function uses a method of pseudo-random number generation that unfortunately emits fairly predictable numbers. + +If the numbers it emits are predictable, then it's obviously not safe to use in cryptographic operations, file name creation, token construction, password generation, and anything else that's related to security. In fact, it may affect security even if it's not directly obvious. + +Switching to a more secure version is simple and the changes look something like this: + +```diff +- import random ++ import secrets + ... +- random.random() ++ secrets.SystemRandom().random() +``` + +If you have feedback on this codemod, [please let us know](mailto:feedback@pixee.ai)! + +## F.A.Q. + +### Why is this codemod marked as Merge After Cursory Review? + +While most of the functions in the `random` module aren't cryptographically secure, there are still valid use cases for `random.random()` such as for simulations or games. + +## Codemod Settings + +N/A + +## References + +- [https://owasp.org/www-community/vulnerabilities/Insecure_Randomness](https://owasp.org/www-community/vulnerabilities/Insecure_Randomness) +- [https://docs.python.org/3/library/random.html](https://docs.python.org/3/library/random.html) +- [Using pseudorandom number generators (PRNGs) is security-sensitive](https://rules.sonarsource.com/python/type/Security%20Hotspot/RSPEC-2245/) diff --git a/docs/codemods/python/sonar_python_secure-tempfile-S5445.md b/docs/codemods/python/sonar_python_secure-tempfile-S5445.md new file mode 100644 index 0000000..9784c58 --- /dev/null +++ b/docs/codemods/python/sonar_python_secure-tempfile-S5445.md @@ -0,0 +1,41 @@ +--- +title: "Sonar: Upgrade and Secure Temp File Creation" +sidebar_position: 1 +--- + +## sonar:python/secure-tempfile-S5445 + +| Importance | Review Guidance | Requires Scanning Tool | +| ---------- | -------------------- | ---------------------- | +| High | Merge Without Review | Yes (Sonar) | + +This codemod acts upon the following Sonar rules: python:S5445. + +This codemod replaces all `tempfile.mktemp` calls to the more secure `tempfile.mkstemp`. + +The Python [tempfile documentation](https://docs.python.org/3/library/tempfile.html#tempfile.mktemp) is explicit +that `tempfile.mktemp` should be deprecated to avoid an unsafe and unexpected race condition. +The changes from this codemod look like this: + +```diff + import tempfile +- tempfile.mktemp(...) ++ tempfile.mkstemp(...) +``` + +If you have feedback on this codemod, [please let us know](mailto:feedback@pixee.ai)! + +## F.A.Q. + +### Why is this codemod marked as Merge Without Review? + +We believe this codemod is safe and will cause no unexpected errors. + +## Codemod Settings + +N/A + +## References + +- [https://docs.python.org/3/library/tempfile.html#tempfile.mktemp](https://docs.python.org/3/library/tempfile.html#tempfile.mktemp) +- [Insecure temporary file creation methods should not be used](https://rules.sonarsource.com/python/type/Vulnerability/RSPEC-5445/) diff --git a/docs/codemods/python/sonar_python_sql-parameterization-S3649.md b/docs/codemods/python/sonar_python_sql-parameterization-S3649.md new file mode 100644 index 0000000..3fa2c95 --- /dev/null +++ b/docs/codemods/python/sonar_python_sql-parameterization-S3649.md @@ -0,0 +1,46 @@ +--- +title: "Sonar: Parameterize SQL Queries" +sidebar_position: 1 +--- + +## sonar:python/sql-parameterization-S3649 + +| Importance | Review Guidance | Requires Scanning Tool | +| ---------- | -------------------------- | ---------------------- | +| High | Merge After Cursory Review | Yes (Sonar) | + +This codemod acts upon the following Sonar rules: pythonsecurity:S3649. + +This codemod refactors SQL statements to be parameterized, rather than built by hand. + +Without parameterization, developers must remember to escape string inputs using the rules for that column type and database. This usually results in bugs -- and sometimes vulnerabilities. Although we can't tell for sure if your code is actually exploitable, this change will make the code more robust in case the conditions which prevent exploitation today ever go away. + +Our changes look something like this: + +```diff +import sqlite3 + +name = input() +connection = sqlite3.connect("my_db.db") +cursor = connection.cursor() +- cursor.execute("SELECT * from USERS WHERE name ='" + name + "'") ++ cursor.execute("SELECT * from USERS WHERE name =?", (name, )) +``` + +If you have feedback on this codemod, [please let us know](mailto:feedback@pixee.ai)! + +## F.A.Q. + +### Why is this codemod marked as Merge After Cursory Review? + +Python has a wealth of database drivers that all use the same `dbapi2` interface detailed in [PEP249](https://peps.python.org/pep-0249/). Different drivers may require different string tokens used for parameterization, and Python's dynamic typing makes it quite hard, and sometimes impossible, to detect which driver is being used just by looking at the code. + +## Codemod Settings + +N/A + +## References + +- [https://cwe.mitre.org/data/definitions/89.html](https://cwe.mitre.org/data/definitions/89.html) +- [https://owasp.org/www-community/attacks/SQL_Injection](https://owasp.org/www-community/attacks/SQL_Injection) +- [Database queries should not be vulnerable to injection attacks](https://rules.sonarsource.com/python/RSPEC-3649/) diff --git a/docs/codemods/python/sonar_python_url-sandbox-S5144.md b/docs/codemods/python/sonar_python_url-sandbox-S5144.md new file mode 100644 index 0000000..5ea48df --- /dev/null +++ b/docs/codemods/python/sonar_python_url-sandbox-S5144.md @@ -0,0 +1,109 @@ +--- +title: "Sonar: Sandbox URL Creation" +sidebar_position: 1 +--- + +## sonar:python/url-sandbox-S5144 + +| Importance | Review Guidance | Requires Scanning Tool | +| ---------- | -------------------------- | ---------------------- | +| High | Merge After Cursory Review | Yes (Sonar) | + +This codemod acts upon the following Sonar rules: pythonsecurity:S5144. + +This codemod sandboxes calls to [`requests.get`](https://requests.readthedocs.io/en/latest/api/#requests.get) to be more resistant to Server-Side Request Forgery (SSRF) attacks. + +Most of the time when you make a `GET` request to a URL, you're intending to reference an HTTP endpoint, like an internal microservice. However, URLs can point to local file system files, a Gopher stream in your local network, a JAR file on a remote Internet site, and all kinds of other unexpected and undesirable outcomes. When the URL values are influenced by attackers, they can trick your application into fetching internal resources, running malicious code, or otherwise harming the system. +Consider the following code for a Flask app: + +```python +from flask import Flask, request +import requests + +app = Flask(__name__) + +@app.route("/request-url") +def request_url(): + url = request.args["loc"] + resp = requests.get(url) + ... +``` + +In this case, an attacker could supply a value like `"http://169.254.169.254/user-data/"` and attempt to access user information. + +Our changes introduce sandboxing around URL creation that force developers to specify some boundaries on the types of URLs they expect to create: + +```diff + from flask import Flask, request +- import requests ++ from security import safe_requests + + app = Flask(__name__) + + @app.route("/request-url") + def request_url(): + url = request.args["loc"] +- resp = requests.get(url) ++ resp = safe_requests.get(url) + ... +``` + +This change alone reduces attack surface significantly because the default behavior of `safe_requests.get` raises a `SecurityException` if +a user attempts to access a known infrastructure location, unless specifically disabled. + +If you have feedback on this codemod, [please let us know](mailto:feedback@pixee.ai)! + +## F.A.Q. + +### Why does this codemod require a Pixee dependency? + +We always prefer to use built-in Python functions or one from a well-known and trusted community dependency. However, we cannot find any such control. If you know of one, [please let us know](https://ask.pixee.ai/feedback). + +### Why is this codemod marked as Merge After Cursory Review? + +By default, the protection only weaves in 2 checks, which we believe will not cause any issues with the vast majority of code: + +1. The given URL must be HTTP/HTTPS. +2. The given URL must not point to a "well-known infrastructure target", which includes things like AWS Metadata Service endpoints, and internal routers (e.g., 192.168.1.1) which are common targets of attacks. + +However, on rare occasions an application may use a URL protocol like "file://" or "ftp://" in backend or middleware code. + +If you want to allow those protocols, change the incoming PR to look more like this and get the best security possible: + +```diff +-resp = requests.get(url) ++resp = safe_requests.get(url, allowed_protocols=("ftp",)) +``` + +If you have feedback on this codemod, [please let us know](mailto:feedback@pixee.ai)! + +## F.A.Q. + +### Why is this codemod marked as Merge After Cursory Review? + +By default, the protection only weaves in 2 checks, which we believe will not cause any issues with the vast majority of code: + +1. The given URL must be HTTP/HTTPS. +2. The given URL must not point to a "well-known infrastructure target", which includes things like AWS Metadata Service endpoints, and internal routers (e.g., 192.168.1.1) which are common targets of attacks. + +However, on rare occasions an application may use a URL protocol like "file://" or "ftp://" in backend or middleware code. + +If you want to allow those protocols, change the incoming PR to look more like this and get the best security possible: + +```diff +-resp = requests.get(url) ++resp = safe_requests.get.get(url, allowed_protocols=("ftp",)) +``` + +## Codemod Settings + +N/A + +## References + +- [https://github.com/pixee/python-security/blob/main/src/security/safe_requests/api.py](https://github.com/pixee/python-security/blob/main/src/security/safe_requests/api.py) +- [https://portswigger.net/web-security/ssrf](https://portswigger.net/web-security/ssrf) +- [https://cheatsheetseries.owasp.org/cheatsheets/Server_Side_Request_Forgery_Prevention_Cheat_Sheet.html](https://cheatsheetseries.owasp.org/cheatsheets/Server_Side_Request_Forgery_Prevention_Cheat_Sheet.html) +- [https://www.rapid7.com/blog/post/2021/11/23/owasp-top-10-deep-dive-defending-against-server-side-request-forgery/](https://www.rapid7.com/blog/post/2021/11/23/owasp-top-10-deep-dive-defending-against-server-side-request-forgery/) +- [https://blog.assetnote.io/2021/01/13/blind-ssrf-chains/](https://blog.assetnote.io/2021/01/13/blind-ssrf-chains/) +- [Server-side requests should not be vulnerable to forging attacks](https://rules.sonarsource.com/python/type/Vulnerability/RSPEC-5144/)