Learn about Python Anti-Patterns and How they help you write better code, and avoid common pitfalls.
The creation of a NumPy array can be done in several ways, for example by passing a Python list to the np.array function.
Another way would be to pass a generator to the np.array function, but doing so creates a 0-dimensional array of objects and may not be the intended goal.
This NumPy array will have a have a data type (dtype) of object and could hold any Python objects.
One of the characteristics of NumPy arrays is homogeneity, meaning all its elements are of the same type. Creating an array of objects allows the user to create heterogeneous array without raising any errors and creating such an array can lead to bugs further in the program.
arr = np.array(x**2 for x in range(10))
arr.reshape(1)
arr.resize(2)
arr.put(indices=1, values=3) # No issues raised.
* and **, conventionally known as *args and **kwargs (although any names can be used), can be used in either a function definition or in a function call. In a function definition, they allow a variable number of arguments to be passed. In a function call, they simply obscure the actual values being used in the call.
my_arg_list = ("yellow", "green", 42)
my_arg_dictionary = {"first" : "Sam", "next": 4, "last": my_obj}
# many lines of code...
function_with_args(*my_arg_list) #Noncompliant
function_with_named_args(**my_arg_dictionary) #Noncompliant
Using reluctant quantifiers (also known as lazy or non-greedy quantifiers) in patterns can often lead to needless backtracking, making the regex needlessly inefficient and potentially vulnerable to catastrophic backtracking. Particularly when using .? or .+? to match anything up to some terminating character, it is usually a better idea to instead use a greedily or quantified negated character class containing the terminating character. For example <.+?> should be replaced with <[^>]> or <[^>]+>.
r'<.+?>'
r'".*?"'
The open builtin function can open files in different modes. These modes are provided as a combination of characters. Using an invalid sequence of characters will make open fail with a ValueError.
A valid mode:
should contain only one of the following characters: r (read), w (write), a (append), x (create).
should contain zero or one of the following characters: t (text), b (binary).
should contain zero or one + character (open for updating)
For example: a, rt, r+ and w+b are valid modes.
If no t or b character is provided the mode will default to t (text), so specifying r is equivalent to rt.
Note: In Python 2, providing an incorrect mode may have an undefined behavior (ex: it might ignore some characters)
with open("test.txt", "aw") as f: # Noncompliant: ValueError
pass
Template engines have an HTML autoescape mechanism that protects web applications against most common cross-site-scripting (XSS) vulnerabilities.
By default, it automatically replaces HTML special characters in any template variables. This secure by design configuration should not be globally disabled.
Escaping HTML from template variables prevents switching into any execution context, like <script>. Disabling autoescaping forces developers to manually escape each template variable for the application to be safe. A more pragmatic approach is to escape by default and to manually disable escaping when needed.
A successful exploitation of a cross-site-scripting vulnerability by an attacker allow him to execute malicious JavaScript code in a user’s web browser. The most severe XSS attacks involve:
Forced redirection
Modify presentation of content
User accounts takeover after disclosure of sensitive information like session cookies or passwords
This rule supports the following libraries:
from jinja2 import Environment
env = Environment() # Noncompliant; New Jinja2 Environment has autoescape set to false
env = Environment(autoescape=False) # Noncompliant
A bare raise statement, i.e. a raise with no exception provided, will re-raise the last active exception in the current scope:
def foo():
try:
...
except ValueError as e:
raise # this will re-raise "e"
yield and return only make sense in the context of functions. Using them outside a function raises a SyntaxError.
If the goal is to break out of a loop, use break instead of return.
a = 1
while a < 3:
if a % 2 == 0:
return # Noncompliant: return is outside of a function
a += 1
for n in range(5):
yield n # Noncompliant: yield is outside of a function
If a function does not return anything, it makes no sense to use its output. Specifically, passing it to another function, or assigning its “result” to a variable is probably a bug because such functions return nothing, which is probably not what was intended.
def foo():
l = [1, 2, 3]
result = l.append(42) # Noncompliant, `append` mutates list l
return result
Using the “locals()” function to pass context to a Django “render()” function can lead to security vulnerabilities and unexpected behavior. “locals()” returns a dictionary of the current local scope, including any sensitive information that may be present in the function’s local namespace. This means that if “locals()” is used to pass context to “render()”, sensitive data such as passwords, keys, and other secrets could be leaked.
Additionally, using “locals()” to pass context can make code more difficult to read and understand. It can also make it harder to maintain code over time.
def my_view(request):
username = "alice"
password = "p@ssw0rd"
context = locals()
return render(request, "my_template.html", context)
Setting a date attribute value with a value which is out of the range of possible values will lead to a ValueError.
def foo():
dt = datetime(year=2024, day=66, month=1, hour=16, minute=1) # ValueError: day is out of range for month
Using pytest.raises or unittest.TestCase.assertRaises will assert that an exception is raised in the following block. Ending such block in an assertion means that the test can succeed with that last assertion never being executed.
import pytest
def foo(): return 1 / 0
def bar(): return 42
def test_something():
with pytest.raises(ZeroDivisionError):
foo()
assert bar() == 42 # Noncompliant
Type hints in Python allow you to specify the expected types of variables and function return values. While type hints are not enforced at runtime, they serve as documentation and can be checked using static type checkers to catch type-related errors during development.
When an assigned value type is incompatible with the type hint of the assigning variable, it can lead to several issues:
Type-related bugs: Assigning a value of an incompatible type to a variable with a specific type hint may lead to unexpected behavior or errors at runtime.
Readability and maintainability: Type hints improve code readability by explicitly stating the intended types of variables and functions. When the assigned value type doesn’t match the hint, it can confuse other developers and make the code harder to maintain.
def my_function():
my_int: int = "string" # Noncompliant
Trying to access a dictionary key that does not exist will raise a KeyError exception.
When trying to access or remove a key that may not be there, several solutions are possible:
Use the get() method instead of a subscription. It will return None for a missing key instead of raising a KeyError.
Use the setdefault() method to provide keys that may be missing with a default value.
Check that the key is present in the dictionary with the if key in dict: construct.
Use a try/except block and handle the potential KeyError exception.
Use a defaultdict instead of a regular dict object, and provide a default_factory attribute.
def foo():
my_dict = {'k1': 42}
...
value = my_dict['k2'] # Noncompliant: the key "k2" does not exist.
Creating property accessors and mutators is quite common in Object-Oriented Programming. Python provides two main ways of implementing getters, setters and deleters; either with the `@property decorator, or with the property function.
`class WithPropertyDecorator(object): @property def foo(self) return self._foo
@foo.setter def foo(self, value): self._foo = value
@foo.deleter def foo(self): del self._foo
class WithPropertyMethod(object): def get_foo(self): return self._foo
def set_foo(self, value): self._foo = value
def del_foo(self): del self._foo
foo = property(get_foo, set_foo, del_foo, “‘foo’ property.”)`
The following program illustrates how using the built-in operations will call the custom methods defined above.
`with_decorator = WithPropertyDecorator() with_decorator.foo = 1 # the method defined under @foo.setter will be called. some_var = with_decorator.foo # the method defined under @foo.getter will be called. del with_decorator.foo # the method defined under @foo.deleter will be called.
with_method = WithPropertyMethod() with_method.foo = 1 # the method set_foo will be called. some_var = with_method.foo # the method get_foo will be called. del with_method.foo # the method del_foo will be called. `
Defining a property this way allows for flexibility when refactoring the implementation of the getters, setters and deleters method, as all the accesses and modifications are done through the Python built-in operators (=,.) and keyword (del).
Property getter, setter and deleter methods are called by the Python interpreter with a specific number of arguments:
Property getter and deleter methods only require a “self” argument.
Property setter methods require a “self” argument as well as a value.
Adding any other parameters, or removing these mandatory parameters will throw a TypeError` exception at runtime when trying to access or modify the property.
class A:
@property
def foo(self, unexpected, unexpected2): # Noncompliant: too many parameters.
return self._foo
@foo.setter
def foo(self): # Noncompliant: a parameter is missing.
self._foo = 42
@foo.deleter
def foo(self, unexpected): # Noncompliant: too many parameters.
del self._foo
class B:
def get_foo(self, unexpected): # Noncompliant: too many parameters.
return self._foo
def set_foo(self, value, unexpected): # Noncompliant: too many parameters.
self._foo = value
def del_foo(self, unexpected): # Noncompliant: too many parameters.
del self._foo
foo = property(get_foo, set_foo, del_foo, "'foo' property.")
The type hint Any represents any possible Python type, on which all operations are possible. This conveys the information that all operations could be possible on a value annotated with Any, making the value dynamically-typed.
Any as a type hint provides no information about the expected type of the variable or parameter, essentially losing the benefits of type hinting. This can make the code less clear and harder to understand and will reduce the code insight capabilities of IDEs and static analysis tools.
def foo(arg: Any) Any:
if isinstance(arg, int):
return arg + 1
else:
return arg.upper()
The pandas library provides an easy way to load data from documents hosted locally or remotely, for example with the pandas.read_csv or pandas.read_table functions:
import pandas as pd
df = pd.read_csv("my_file.csv")
When the names of parameters in a function/method call match the names of the method arguments, it contributes to clearer, more readable code. However, when the names match, but are passed in a different order than the function/method arguments, it indicates a mistake in the parameter order which will likely lead to unexpected results.
def move_point(coord, speed):
new_x = coord[0] + speed[0]
new_y = coord[1] + speed[1]
return (new_x, new_y)
coord = (3, 4)
speed = (1, 2)
move_point(speed, coord) # Noncompliant
Exceptions handlers (except) are evaluated in the order they are written. Once a match is found, the evaluation stops.
In some contexts, an except block is dead code as it will never catch any exception:
If there is a handler for a base class followed by a handler for class derived from that base class, the second handler will never trigger: The handler for the base class will match the derived class, and will be the only executed handler.
When multiple except statements try to catch the same exception class, only the first one will be executed.
In Python 3, BaseException is the parent of every exception class. When a BaseException is caught by an except clause, none of the subsequent except statement will catch anything. This is true as well for the bare except statement (except:).
def foo():
try:
raise FloatingPointError()
except (ArithmeticError, RuntimeError) as e:
print(e)
except FloatingPointError as e: # Noncompliant: FloatingPointError is a subclass of ArithmeticError.
print("Never executed")
except OverflowError as e: # Noncompliant: OverflowError is a subclass of ArithmeticError.
print("Never executed")
try:
raise TypeError()
except TypeError as e:
print(e)
except TypeError as e: # Noncompliant: duplicate except.
print("Never executed")
try:
raise ValueError()
except BaseException as e:
print(e)
except: # Noncompliant: this is equivalent to "except BaseException" block.
print("Never executed")
Importing names and not using them can be a source of confusion and lead to maintainability issues.
Such imports should be removed.
from mymodule import foo, bar, qix # Noncompliant: bar is unused
foo()
qix()
utbound communications can lead to data leaks.
A restrictive security group is an additional layer of protection that might prevent the abuse or exploitation of a resource. For example, it complicates the exfiltration of data in the case of a successfully exploited vulnerability.
When deciding if outgoing connections should be limited, consider that limiting the connections results in additional administration and maintenance work.
from aws_cdk import (
aws_ec2 as ec2
)
ec2.SecurityGroup( # Sensitive; allow_all_outbound is enabled by default
self,
"example",
vpc=vpc
)
In Python 2, backticks are a deprecated alias for repr(). The syntax was removed in Python 3. To make the transition to Python 3 easier, they should not be used anymore.
return `num` # Noncompliant
Python concatenates adjacent string or byte literals at compile time. It means that “a” “b” is equivalent to “ab”.
This is sometimes used to split a long string on multiple lines. However an implicit string concatenation can also be very confusing.
In the following contexts it might indicate that a comma was forgotten:
when the two strings are on the same line. This could be interpreted as an incorrectly formatted tuple (parentheses are not mandatory to create a tuple, only the comma is).
when the strings are in a list, a set or a tuple.
def func():
return "item1" "item2" # Noncompliant: a comma is missing to return a tuple.
["1" # Noncompliant: a comma is missing.
"2",
"a very" # Noncompliant: a "+" is missing.
"long string"]
A format string is a string that contains placeholders, usually represented by special characters such as “%s” or "", depending on the technology in use. These placeholders are replaced by values when the string is printed or logged. Thus, it is required that a string is valid and arguments match replacement fields in this string.
This applies to the % operator, the str.format method, and loggers from the logging module. Internally, the latter use the %-formatting. The only difference is that they will log an error instead of raising an exception when the provided arguments are invalid.
Formatted string literals (also called “f-strings”; available since Python 3.6) are generally simpler to use, and any syntax mistake will cause a failure at compile time. However, it is easy to forget curly braces, which will not lead to any detectable errors.
This rule raises an issue when:
A string formatted with % will not return the expected text because some arguments are unused.
A string formatted with str.format will not return the expected string because some arguments are unused.
An “f-string” doesn’t contain any replacement field, which probably means some curly braces are missing.
Loggers will log an error because their message is not formatted properly.
Rule S2275 covers cases where formatting a string will raise an exception.
"Error %(message)s" % {"message": "something failed", "extra": "some dead code"} # Noncompliant. Remove the unused argument "extra" or add a replacement field.
"Error: User {} has not been able to access []".format("Alice", "MyFile") # Noncompliant. Remove 1 unexpected argument or add a replacement field.
user = "Alice"
resource = "MyFile"
message = f"Error: User [user] has not been able to access [resource]" # Noncompliant. Add replacement fields or use a normal string instead of an f-string.
import logging
logging.error("Error: User %s has not been able to access %s", "Alice") # Noncompliant. Add 1 missing argument.
Global variables are a useful construct, but they should not be abused. Functions access the global scope through the global keyword, but this practice considerably reduce the function’s readability and reusability. Instead, the global variable should be passed as a parameter to the function.
NAME = 'Joe'
def write_name();
global NAME # Noncompliant
print NAME
During dictionary use, it is normal that you might want to replace the value associated with one of the dictionary’s keys. But do so during the initial definition of a dictionary, and you’ve probably made an error.
This rule raises an issue when the same key is used multiple times in the initial definition of a dictionary.
fruit = {"apple":4, "pear":6, "quince":29, "apple": 8} # Noncompliant; "apple" is used in positions 0 and 3
In Django, the ‘@receiver’ decorator is used to register signal handlers. These handlers are used to respond to events that occur in the application, such as a user logging in or a database record being saved.
The order in which decorators are applied can have a significant impact on their behavior. In the case of the @receiver decorator, it is important that it is applied first, before any other decorators, in order to ensure that the signal handler is registered correctly.
If the ‘@receiver’ decorator is not applied first, the decorators placed above it will be ignored, which can result in unexpected behavior or even errors in the application.
from django.dispatch import receiver
from django.views.decorators.csrf import csrf_exempt
@csrf_exempt
@receiver(some_signal)
def my_handler(sender, **kwargs):
...
The `unittest module provides assertion methods specific to common types and operations. Both versions will test the same things, but the dedicated one will provide a better error message, simplifying the debugging process.
This rule reports an issue when an assertion can be simplified by using a more specific function. The array below gives a list of assertions on which an issue will be raised, and which function should be used instead:
import unittest
class SomeTest(unittest.TestCase):
def test_something(self):
x = foo()
y = bar()
self.assertFalse(x == y) # Noncompliant
self.assertTrue(x < y) # Noncompliant
Python interprets False as a “false” boolean value, and True as a “true” boolean value. Assigning them other values could lead to unexpected behaviors.
This rule raises an issue when “True” and “False”, with any case, are used as identifiers.
True = "red" # Noncompliant
Variables, Classes and functions should be defined before they are used, otherwise the code will fail.
def noncompliant():
foo() # Noncompliant
foo = sum
func() # Noncompliant
def func():
pass
MyClass() # Noncompliant
class MyClass:
pass
None is a built-in object that represents the absence of a value. It is often used as a placeholder value for variables that only sometimes hold a value or as a return value for method calls that have no result.
Attributes and methods of symbols that sometimes can be None should only be accessed in circumstances where it is certain that they are not set to None. Otherwise, an AttributeError is raised, and the program is interrupted. Hence, this issue indicates a logical error as it results from incorrect assumptions about the state of variables or the results of computations.
def render(file_path):
if os.path.isfile(file_path):
data = interpret_csv(file_path)
else:
data = None
# ...
data.plot_graph() # Noncompliant
The CPython interpreter does not check types of arguments when functions are called. However, a function can express the type it expects for each argument in its documentation or by using Type Hints. While the code may initially work as intended, not respecting the contract of an API may lead to bugs later when its implementation evolves or when type checks are added (i.e. with isinstance).
This rule also checks argument types for built-in functions.
def func(var: str):
pass
func(42) # Noncompliant: 42 is not of type str.
round("not a number") # Noncompliant: the builtin function round requires a number as first parameter.
You’re allowed to make function calls with positionally-identified parameters or with named parameters. In long parameter lists, explicitly naming your parameters both eliminates the risk of swapping parameter values and makes the call clearer to maintainers.
def coordinateSurpriseParty(self, hostName, guestOfHonor, caterer, invitees, peopleNotToInvite):
# ...
host="John"
guestOfHonor="Fred"
caterer="Barry"
invitees="Thea, Will, Mark, Mary"
peopleToExclude="Susan, Alberta, Frank, Larry"
coordinateSurpriseParty(caterer, host, guestOfHonor, peopleToExclude, invitees) # Noncompliant; this party will be a train wreck!
The print statement was removed in Python 3.0. The built-in function should be used instead.
print '1' # Noncompliant
Because it is dynamically typed, Python does not enforce a return type on a function. This means that different paths through a function can return different types of values, which can be very confusing to the user and significantly harder to maintain.
In particular, it is consequently also possible to mix empty return statements (implicitly returning None) with some returning an expression. This rule verifies that all the return statements from a function are consistent.
def foo(a): # Noncompliant, function will return "true" or None
if a == 1:
return True
return
When a server receives a deeply nested query, it attempts to resolve all the requested data. This process can consume a substantial amount of computational resources, leading to a slowdown in server response times.
from graphql_server.flask import GraphQLView
app.add_url_rule("/api",
view_func=GraphQLView.as_view( # Noncompliant
name="api",
schema=schema,
)
)
When a test fails due, for example, to infrastructure issues, you might want to ignore it temporarily. But without some kind of notation about why the test is being ignored, it may never be reactivated. Such tests are difficult to address without comprehensive knowledge of the project, and end up polluting their projects.
This rule raises an issue for each skipped test with “unittest.skip” or “pytest.mark.skip” without providing a reason argument.
import unittest
class MyTest(unittest.TestCase):
@unittest.skip # Noncompliant
def test_something(self): ...
Using `return, break or continue in a finally block suppresses the propagation of any unhandled exception which was raised in the try, else or except blocks. It will also ignore their return statements.
SystemExit is raised when sys.exit() is called. KeyboardInterrupt is raised when the user asks the program to stop by pressing interrupt keys. Both exceptions are expected to propagate up until the application stops. It is ok to catch them when a clean-up is necessary but they should be raised again immediately. They should never be ignored.
If you need to ignore every other exception you can simply catch the Exception class. However you should be very careful when you do this as it will ignore other important exceptions such as MemoryError
In python 2 it is possible to raise old style classes. You can use a bare except: statement to catch every exception. Remember to still reraise SystemExit and KeyboardInterrupt.
This rule raises an issue when a jump statement (break, continue, return`) would force the control flow to leave a finally block.
def find_file_which_contains(expected_content, paths):
file = None
for path in paths:
try:
# "open" will raise IsADirectoryError if the provided path is a directory but it will be stopped by the "return" and "continue"
file = open(path, 'r')
actual_content = file.read()
except FileNotFoundError as exception:
# This exception will never pass the "finally" block because of "return" and "continue"
raise ValueError(f"'paths' should only contain existing files. File ${path} does not exist.")
finally:
file.close()
if actual_content != expected_content:
# Note that "continue" is allowed in a "finally" block only since python 3.8
continue # Noncompliant. This will prevent exceptions raised by the "try" block and "except" block from raising.
else:
return path # Noncompliant. Same as for "continue"
return None
# This will return None instead of raising ValueError from the "except" block
find_file_which_contains("some content", ["file_which_does_not_exist"])
# This will return None instead of raising IsADirectoryError from the "try" block
find_file_which_contains("some content", ["a_directory"])
import sys
while True:
try:
sys.exit(1)
except (SystemExit) as e:
print("Exiting")
raise
finally:
break # This will prevent SystemExit from raising
def continue_whatever_happens_noncompliant():
for i in range(10):
try:
raise ValueError()
finally:
continue # Noncompliant
Since Python 3.12 the keyword type is used to defined type aliases. It replaces the following construct:
from typing import TypeAlias, TypeVar
_T = TypeVar("_T")
MyTypeAlias: TypeAlias = set[_T]
When a class or function is defined in a parent function or method, it is only visible in this parent function or method’s scope. If the defined class or function is not used within this scope it is dead code (unnecessary, inoperative code) that should be removed.
Cleaning out dead code decreases the size of the maintained codebase, making it easier to understand the program and preventing bugs from being introduced.
def parent_function():
def nested_function(): # Noncompliant: this function is never used in this scope.
print("nested_function")
class NestedClass: # Noncompliant: this class is never used in this scope.
def __init__(self):
print("NestedClass")
The use of a pass statement where it is not required by the syntax is redundant. It makes the code less readable and its intent confusing.
To fix this issue, remove pass statements that do not affect the behaviour of the program.
def foo(arg):
print(arg)
pass # Noncompliant: the `pass` statement is not needed as it does not change the behaviour of the program.
The %p directive in the strftime method is used to represent the AM/PM marker in a time string. It is commonly used in conjunction with the %I directive, which represents the hour in a 12-hour clock.
Using the 24-hour format directive with an AM/PM marker can lead to unwanted results e.g.:
time_string = time(16,0).strftime("%H:%M %p")
print(time_string)
It is needlessly complex to invert the result of a boolean comparison. The opposite comparison should be made instead.
if not a == 2: # Noncompliant
b = not i < 10 # Noncompliant
Regular expressions have their own syntax that is understood by regular expression engines. Those engines will throw an exception at runtime if they are given a regular expression that does not conform to that syntax.
To avoid syntax errors, special characters should be escaped with backslashes when they are intended to be matched literally and references to capturing groups should use the correctly spelled name or number of the group.
re.compile(r"([")
re.sub(r"([", input, "{")
re.compile(r"(\w+-(\d+)")
Operating systems have global directories where any user has write access. Those folders are mostly used as temporary storage areas like `/tmp in Linux based systems. An application manipulating files from these folders is exposed to race conditions on filenames: a malicious user can try to create a file with a predictable name before the application does. A successful attack can result in other files being accessed, modified, corrupted or deleted. This risk is even higher if the application runs with elevated permissions.
In the past, it has led to the following vulnerabilities:
This rule raises an issue whenever it detects a hard-coded path to a publicly writable directory like /tmp (see examples bellow). It also detects access to environment variables that point to publicly writable directories, e.g., TMP and TMPDIR.
/tmp
/var/tmp
/usr/tmp
/dev/shm
/dev/mqueue
/run/lock
/var/run/lock
/Library/Caches
/Users/Shared
/private/tmp
/private/var/tmp
\Windows\Temp
\Temp
\TMP`
import tempfile
file = tempfile.TemporaryFile(dir="/tmp/my_subdirectory", mode="w+") # Compliant
Sub-patterns can be wrapped by parentheses to build a group. This enables to restrict alternations, back reference the group or apply quantifier to the sub-pattern.
If this group should not be part of the match result or if no reference to this group is required, a non-capturing group can be created by adding ?: behind the opening parenthesis.
However, if this non-capturing group does not have a quantifier, or does not wrap an alternation, then imaging this group is redundant.
r"(?:number)\d{2}"
Curly brace quantifiers in regular expressions can be used to have a more fine-grained control over how many times the character or the sub-expression preceeding them should occur. They can be used to match an expression exactly n times with `, between n and m times with , or at least n times with n. In some cases, using such a quantifier is superfluous for the semantic of the regular expression, and it can be removed to improve readability. This rule raises an issue when one of the following quantifiers is encountered:
or : they match the expression exactly once. The same behavior can be achieved without the quantifier.
or `: they match the expression zero times. The same behavior can be achieved by removing the expression.
r"ab{1,1}c"
r"ab{1}c"
r"ab{0,0}c"
r"ab{0}c"
Putting multiple statements on a single line lowers the code readability and makes debugging the code more complex.
Unresolved directive in <stdin> - include::[]
Write one statement per line to improve readability.
Unresolved directive in <stdin> - include::[]
if (True): print("hello") # Noncompliant
Resource-based policies granting access to all users can lead to information leakage.
from aws_cdk.aws_iam import PolicyStatement, AnyPrincipal, Effect
from aws_cdk.aws_s3 import Bucket
bucket = Bucket(self, "ExampleBucket")
bucket.add_to_resource_policy(PolicyStatement(
effect=Effect.ALLOW,
actions=["s3:*"],
resources=[bucket.arn_for_objects("*")],
principals=[AnyPrincipal()] # Sensitive
))
A cross-site request forgery (CSRF) attack occurs when a trusted user of a web application can be forced, by an attacker, to perform sensitive actions that he didn’t intend, such as updating his profile or sending a message, more generally anything that can change the state of the application.
The attacker can trick the user/victim to click on a link, corresponding to the privileged action, or to visit a malicious web site that embeds a hidden web request and as web browsers automatically include cookies, the actions can be authenticated and sensitive.
MIDDLEWARE = [
'django.middleware.security.SecurityMiddleware',
'django.contrib.sessions.middleware.SessionMiddleware',
'django.middleware.common.CommonMiddleware',
'django.middleware.csrf.CsrfViewMiddleware', # Compliant
'django.contrib.auth.middleware.AuthenticationMiddleware',
'django.contrib.messages.middleware.MessageMiddleware',
'django.middleware.clickjacking.XFrameOptionsMiddleware',
]
Alternation is used to match a single regular expression out of several possible regular expressions. If one of the alternatives is empty it would match any input, which is most probably a mistake.
re.search(r"Jack|Peter|", "John") # Noncompliant - will match an empty string
re.search(r"Jack||Peter", "John") # Noncompliant - will match an empty string
By default, S3 buckets can be accessed through HTTP and HTTPs protocols.
As HTTP is a clear-text protocol, it lacks the encryption of transported data, as well as the capability to build an authenticated connection. It means that a malicious actor who is able to intercept traffic from the network can read, modify or corrupt the transported content.
import aws_cdk.aws_s3 as s3
import aws_cdk.aws_iam as iam
bucket = s3.Bucket(self, "bucket") # Sensitive
When executing an OS command and unless you specify the full path to the executable, then the locations in your application’s PATH environment variable will be searched for the executable. That search could leave an opening for an attacker if one of the elements in PATH is a directory under his control.
params = ["/usr/bin/binary", "arg"]
subprocess.call(params) # Compliant
Having a permissive Cross-Origin Resource Sharing policy is security-sensitive. It has led in the past to the following vulnerabilities:
Same origin policy in browsers prevents, by default and for security-reasons, a javascript frontend to perform a cross-origin HTTP request to a resource that has a different origin (domain, protocol, or port) from its own. The requested target can append additional HTTP headers in response, called CORS, that act like directives for the browser and change the access control policy / relax the same origin policy.
origin = request.headers['ORIGIN']
resp = Response()
resp.headers['Access-Control-Allow-Origin'] = origin # Sensitive
Character classes in regular expressions are a convenient way to match one of several possible characters by listing the allowed characters or ranges of characters. If a character class contains only one character, the effect is the same as just writing the character without a character class.
Thus, having only one character in a character class is usually a simple oversight that remained after removing other characters of the class.
r"a[b]c"
def swap(mylist, index1, index2):
tmp = mylist[index2]
mylist[index2] = mylist[index1]
mylist[index2] = tmp # Noncompliant
list2 = [0,1,2,3,4,5,6,7,8,9]
list2[3:5] = [42,42]
list2[3:5] = [42,42] # Noncompliant
mymap = {'a': {}}
mymap['a']['b'] = 42
mymap['a']['b'] = 42 # Noncompliant
Enabling public network access to cloud resources can affect an organization’s ability to protect its data or internal operations from data theft or disruption.
Depending on the component, inbound access from the Internet can be enabled via:
a boolean value that explicitly allows access to the public network.
the assignment of a public IP address.
database firewall rules that allow public IP ranges.
Deciding to allow public access may happen for various reasons such as for quick maintenance, time saving, or by accident.
This decision increases the likelihood of attacks on the organization, such as:
data breaches.
intrusions into the infrastructure to permanently steal from it.
and various malicious traffic, such as DDoS attacks.
from aws_cdk import aws_ec2 as ec2
ec2.Instance(
self,
"vpc_subnet_public",
instance_type=nano_t2,
machine_image=ec2.MachineImage.latest_amazon_linux(),
vpc=vpc,
vpc_subnets=ec2.SubnetSelection(subnet_type=ec2.SubnetType.PUBLIC) # Sensitive
)
Using unencrypted RDS DB resources exposes data to unauthorized access. This includes database data, logs, automatic backups, read replicas, snapshots, and cluster metadata.
This situation can occur in a variety of scenarios, such as:
A malicious insider working at the cloud provider gains physical access to the storage device.
Unknown attackers penetrate the cloud provider’s logical infrastructure and systems.
After a successful intrusion, the underlying applications are exposed to:
theft of intellectual property and/or personal data
extortion
denial of services and security bypasses via data corruption or deletion
AWS-managed encryption at rest reduces this risk with a simple switch.
from aws_cdk import (
aws_rds as rds
)
class DatabaseStack(Stack):
def __init__(self, scope: Construct, construct_id: str, **kwargs) None:
super().__init__(scope, construct_id, **kwargs)
rds.DatabaseCluster( # Sensitive, unencrypted by default
self,
"example"
)
A general catch block seems like an efficient way to handle multiple possible exceptions. Unfortunately, it traps all exception types, casting too broad a net, and perhaps mishandling extraordinary cases. Instead, specific exception sub-types should be caught.
try:
result = do_the_thing(a)
except: #Noncompliant; no specific exception caught
result = None
try:
result = do_the_thing(a)
except Exception: #Noncompliant; too generic to be the only exception type caught
result = None
Development tools and frameworks usually have options to make debugging easier for developers. Although these features are useful during development, they should never be enabled for applications deployed in production. Debug instructions or error messages can leak detailed information about the system, like the application’s path or file names.
from django.conf import settings
settings.configure(DEBUG=True) # Sensitive when set to True
settings.configure(DEBUG_PROPAGATE_EXCEPTIONS=True) # Sensitive when set to True
def custom_config(config):
settings.configure(default_settings=config, DEBUG=True) # Sensitive
Amazon SageMaker is a managed machine learning service in a hosted production-ready environment. To train machine learning models, SageMaker instances can process potentially sensitive data, such as personal information that should not be stored unencrypted. In the event that adversaries physically access the storage media, they cannot decrypt encrypted data.
from aws_cdk import (
aws_sagemaker as sagemaker
)
class CfnSagemakerStack(Stack):
def __init__(self, scope: Construct, construct_id: str, **kwargs) None:
super().__init__(scope, construct_id, **kwargs)
sagemaker.CfnNotebookInstance(
self, "Sensitive",
instance_type="instanceType",
role_arn="roleArn"
) # Sensitive, no KMS key is set by default; thus, encryption is disabled
When exceptions occur, it is usually a bad idea to simply ignore them. Instead, it is better to handle them properly, or at least to log them.
# pass
try:
# ...
except:
pass # Noncompliant
# continue
for i in tab:
try:
# ...
except:
continue # Noncompliant
Successful Zip Bomb attacks occur when an application expands untrusted archive files without controlling the size of the expanded data, which can lead to denial of service. A Zip bomb is usually a malicious archive file of a few kilobytes of compressed data but turned into gigabytes of uncompressed data. To achieve this extreme compression ratio, attackers will compress irrelevant data (eg: a long string of repeated bytes).
import tarfile
THRESHOLD_ENTRIES = 10000
THRESHOLD_SIZE = 1000000000
THRESHOLD_RATIO = 10
totalSizeArchive = 0;
totalEntryArchive = 0;
tfile = tarfile.open("TarBomb.tar")
for entry in tfile:
tarinfo = tfile.extractfile(entry)
totalEntryArchive += 1
sizeEntry = 0
result = b''
while True:
sizeEntry += 1024
totalSizeArchive += 1024
ratio = sizeEntry / entry.size
if ratio > THRESHOLD_RATIO:
# ratio between compressed and uncompressed data is highly suspicious, looks like a Zip Bomb Attack
break
chunk = tarinfo.read(1024)
if not chunk:
break
result += chunk
if totalEntryArchive > THRESHOLD_ENTRIES:
# too much entries in this archive, can lead to inodes exhaustion of the system
break
if totalSizeArchive > THRESHOLD_SIZE:
# the uncompressed data size is too much for the application resource capacity
break
tfile.close()
An HTTP method is safe when used to perform a read-only operation, such as retrieving information. In contrast, an unsafe HTTP method is used to change the state of an application, for instance to update a user’s profile on a web application.
Common safe HTTP methods are GET, HEAD, or OPTIONS.
Common unsafe HTTP methods are POST, PUT and DELETE.
Allowing both safe and unsafe HTTP methods to perform a specific operation on a web application could impact its security, for example CSRF protections are most of the time only protecting operations performed by unsafe HTTP methods.
@require_http_methods(["POST"])
def view(request):
return HttpResponse("...")
A policy that grants all permissions may indicate an improper access control, which violates the principle of least privilege. Suppose an identity is granted full permissions to a resource even though it only requires read permission to work as expected. In this case, an unintentional overwriting of resources may occur and therefore result in loss of information.
from aws_cdk.aws_iam import PolicyStatement, Effect
PolicyStatement(
effect=Effect.ALLOW,
actions=["*"], # Sensitive
resources=["arn:aws:iam:::user/*"]
)
Nested code - blocks of code inside blocks of code - is eventually necessary, but increases complexity. This is why keeping the code as flat as possible, by avoiding unnecessary nesting, is considered a good practice.
Merging if statements when possible will decrease the nesting of the code and improve its readability.
if condition1:
if condition2: # Noncompliant
# ...
An empty is generally considered bad practice and can lead to confusion, readability, and maintenance issues. Empty s bring no functionality and are misleading to others as they might think the implementation fulfills a specific and identified requirement.
There are several reasons for a not to have a body:
It is an unintentional omission, and should be fixed to prevent an unexpected behavior in production.
It is not yet, or never will be, supported. In this case an exception should be thrown.
The method is an intentionally-blank override. In this case a nested comment should explain the reason for the blank override.
import abc
class MyAbstractClass(abc.ABC):
@abc.abstractproperty
def myproperty(self):
pass
@abc.abstractclassmethod
def myclassmethod(cls):
pass
@abc.abstractmethod
def mymethod(self):
pass
@abc.abstractstaticmethod
def mystaticmethod():
pass
S3 buckets can be in three states related to versioning:
unversioned (default one)
enabled
suspended
When the S3 bucket is unversioned or has versioning suspended it means that a new version of an object overwrites an existing one in the S3 bucket.
It can lead to unintentional or intentional information loss.
bucket = s3.Bucket(self, "bucket",
versioned=False # Sensitive
)
Lookahead assertions are a regex feature that makes it possible to look ahead in the input without consuming it. It is often used at the end of regular expressions to make sure that substrings only match when they are followed by a specific pattern.
For example, the following pattern will match an “a” only if it is directly followed by a “b”. This does not consume the “b” in the process:
Unresolved directive in <stdin> - include::[]
However, lookaheads can also be used in the middle (or at the beginning) of a regex. In that case there is the possibility that what comes after the lookahead contradicts the pattern inside the lookahead. Since the lookahead does not consume input, this makes the lookahead impossible to match and is a sign that there’s a mistake in the regular expression that should be fixed.
r"(?=a)b" # Noncompliant, the same character can't be equal to 'a' and 'b' at the same time
Clear-text protocols such as `ftp, telnet, or http lack encryption of transported data, as well as the capability to build an authenticated connection. It means that an attacker able to sniff traffic from the network can read, modify, or corrupt the transported content. These protocols are not secure as they expose applications to an extensive range of risks:
sensitive data exposure
traffic redirected to a malicious endpoint
malware-infected software update or installer
execution of client-side code
corruption of critical information
Even in the context of isolated networks like offline environments or segmented cloud environments, the insider threat exists. Thus, attacks involving communications being sniffed or tampered with can still happen.
For example, attackers could successfully compromise prior security layers by:
bypassing isolation mechanisms
compromising a component of the network
getting the credentials of an internal IAM account (either from a service account or an actual person)
In such cases, encrypting communications would decrease the chances of attackers to successfully leak data or steal credentials from other network components. By layering various security practices (segmentation and encryption, for example), the application will follow the defense-in-depth principle.
Note that using the http` protocol is being deprecated by major web browsers.
In the past, it has led to the following vulnerabilities:
url = "http://example.com" # Sensitive
url = "ftp://anonymous@example.com" # Sensitive
url = "telnet://anonymous@example.com" # Sensitive
import telnetlib
cnx = telnetlib.Telnet("towel.blinkenlights.nl") # Sensitive
import ftplib
cnx = ftplib.FTP("ftp.example.com") # Sensitive
import smtplib
smtp = smtplib.SMTP("smtp.example.com", port=587) # Sensitive
Each source file should start with a header stating file ownership and the license which must be used to distribute the application.
This rule must be fed with the header text that is expected at the beginning of every file.
#
# SonarQube, open source software quality management tool.
# Copyright (C) 2008-2018 SonarSource
# mailto:contact AT sonarsource DOT com
#
# SonarQube is free software; you can redistribute it and/or
# modify it under the terms of the GNU Lesser General Public
# License as published by the Free Software Foundation; either
# version 3 of the License, or (at your option) any later version.
#
# SonarQube is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
# Lesser General Public License for more details.
#
# You should have received a copy of the GNU Lesser General Public License
# along with this program; if not, write to the Free Software Foundation,
# Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
#
When a cookie is configured with the HttpOnly attribute set to true, the browser guaranties that no client-side script will be able to read it. In most cases, when a cookie is created, the default value of HttpOnly is false and it’s up to the developer to decide whether or not the content of the cookie can be read by the client-side script. As a majority of Cross-Site Scripting (XSS) attacks target the theft of session-cookies, the HttpOnly attribute can help to reduce their impact as it won’t be possible to exploit the XSS vulnerability to steal session-cookies.
from flask import Response
@app.route('/')
def index():
response = Response()
response.set_cookie('key', 'value', httponly=True) # Compliant
return response
When a reluctant (or lazy) quantifier is followed by a pattern that can match the empty string or directly by the end of the regex, it will always match zero times for *? or one time for +?. If a reluctant quantifier is followed directly by the end anchor ($), it behaves indistinguishably from a greedy quantifier while being less efficient.
This is likely a sign that the regex does not work as intended.
re.replace(r"start\w*?(end)?", "x", "start123endstart456") # Noncompliant. In contrast to what one would expect, the result is not "xx"
re.match(r"^\d*?$", "123456789") # Noncompliant. Matches the same as "/^\d*$/", but will backtrack in every position.
A regular expression is a sequence of characters that specifies a match pattern in text. Among the most important concepts are:
Character classes: defines a set of characters, any one of which can occur in an input string for a match to succeed.
Quantifiers: used to specify how many instances of a character, group, or character class must be present in the input for a match.
Wildcard (.): matches all characters except line terminators (also matches them if the s flag is set).
Many of these features include shortcuts of widely used expressions, so there is more than one way to construct a regular expression to achieve the same results. For example, to match a two-digit number, one could write [0-9] or \d. The latter is not only shorter but easier to read and thus to maintain.
This rule recommends replacing some quantifiers and character classes with more concise equivalents:
\d for [0-9] and \D for [^0-9]
\w for [A-Za-z0-9_] and \W for `[^A-Za-z0-9_]
. for character classes matching everything (e.g. [\w\W], [\d\D], or [\s\S] with s flag)
x? for x0,1, x* for x0, x+ for x1, xN for xN,N
r"[0-9]" # Noncompliant - same as r"\d"
r"[^0-9]" # Noncompliant - same as r"\D"
r"[A-Za-z0-9_]" # Noncompliant - same as r"\w"
r"[\w\W]" # Noncompliant - same as r"."
r"a{0,}" # Noncompliant - same as r"a*"
Server-side encryption (SSE) encrypts an object (not the metadata) as it is written to disk (where the S3 bucket resides) and decrypts it as it is read from disk. This doesn’t change the way the objects are accessed, as long as the user has the necessary permissions, objects are retrieved as if they were unencrypted. Thus, SSE only helps in the event of disk thefts, improper disposals of disks and other attacks on the AWS infrastructure itself.
There are three SSE options:
Server-Side Encryption with Amazon S3-Managed Keys (SSE-S3)
AWS manages encryption keys and the encryption itself (with AES-256) on its own.
Server-Side Encryption with Customer Master Keys (CMKs) Stored in AWS Key Management Service (SSE-KMS)
AWS manages the encryption (AES-256) of objects and encryption keys provided by the AWS KMS service.
Server-Side Encryption with Customer-Provided Keys (SSE-C)
AWS manages only the encryption (AES-256) of objects with encryption keys provided by the customer. AWS doesn’t store the customer’s encryption keys.
bucket = s3.Bucket(self,"bucket",
encryption=s3.BucketEncryption.UNENCRYPTED # Sensitive
)
Arbitrary OS command injection vulnerabilities are more likely when a shell is spawned rather than a new process, indeed shell meta-chars can be used (when parameters are user-controlled for instance) to inject OS commands.
# by default shell=False, a shell is not spawn
subprocess.run(cmd) # Compliant
subprocess.Popen(cmd) # Compliant
subprocess.call(cmd) # Compliant
subprocess.check_call(cmd) # Compliant
subprocess.check_output(cmd) # Compliant
# always in a subprocess:
os.spawnl(mode, path, *cmd) # Compliant
os.spawnle(mode, path, *cmd, env) # Compliant
os.spawnlp(mode, file, *cmd) # Compliant
os.spawnlpe(mode, file, *cmd, env) # Compliant
os.spawnv(mode, path, cmd) # Compliant
os.spawnve(mode, path, cmd, env) # Compliant
os.spawnvp(mode, file, cmd) # Compliant
os.spawnvpe(mode, file, cmd, env) # Compliant
(child_stdout) = os.popen(cmd, mode, 1) # Compliant
(_, output) = subprocess.getstatusoutput(cmd) # Compliant
out = subprocess.getoutput(cmd) # Compliant
os.startfile(path) # Compliant
os.execl(path, *cmd) # Compliant
os.execle(path, *cmd, env) # Compliant
os.execlp(file, *cmd) # Compliant
os.execlpe(file, *cmd, env) # Compliant
os.execv(path, cmd) # Compliant
os.execve(path, cmd, env) # Compliant
os.execvp(file, cmd) # Compliant
os.execvpe(file, cmd, env) # Compliant
A chain of if/else if statements is evaluated from top to bottom. At most, only one branch will be executed: the first one with a condition that evaluates to true.
Therefore, duplicating a condition automatically leads to dead code. Usually, this is due to a copy/paste error. At best, it’s simply dead code and at worst, it’s a bug that is likely to induce further bugs as the code is maintained, and obviously it could lead to unexpected behavior.
if param == 1:
openWindow()
elif param == 2:
closeWindow()
elif param == 1: # Noncompliant
moveWindowToTheBackground()
Amazon Simple Notification Service (SNS) is a managed messaging service for application-to-application (A2A) and application-to-person (A2P) communication. SNS topics allows publisher systems to fanout messages to a large number of subscriber systems. Amazon SNS allows to encrypt messages when they are received. In the case that adversaries gain physical access to the storage medium or otherwise leak a message they are not able to access the data.
from aws_cdk import (
aws_sns as sns
)
class TopicStack(Stack):
def __init__(self, scope: Construct, construct_id: str, **kwargs) None:
super().__init__(scope, construct_id, **kwargs)
sns.Topic( # Sensitive, unencrypted by default
self,
"example"
)
In regular expressions the boundaries `^ and A can only match at the beginning of the input (or, in case of ^ in combination with the MULTILINE flag, the beginning of the line) and $, \Z and \z only at the end.
These patterns can be misused, by accidentally switching ^ and $` for example, to create a pattern that can never match.
# This can never match because $ and ^ have been switched around
r"$[a-z]+^" # Noncompliant
In regular expressions, anchors (^, ,A,Zandz)havehigherprecedencethanthe∣operator.Soinaregularexpressionlikealt1∣alt2∣alt3, alt1 would be anchored to the beginning, alt3 to the end and alt2 wouldn’t be anchored at all. Usually the intended behavior is that all alternatives are anchored at both ends. To achieve this, a non-capturing group should be used around the alternatives.
In cases where it is intended that the anchors only apply to one alternative each, adding (non-capturing) groups around the anchors and the parts that they apply to will make it explicit which parts are anchored and avoid readers misunderstanding the precedence or changing it because they mistakenly assume the precedence was not intended.
r"^a|b|c$"
Formatted SQL queries can be difficult to maintain, debug and can increase the risk of SQL injection when concatenating untrusted values into the query. However, this rule doesn’t detect SQL injections (unlike rule S3649), the goal is only to highlight complex/formatted queries.
cursor = connection.cursor(prepared=True)
sql_insert_query = """ select col from sometable here mycol = %s and othercol = %s """
select_tuple = (1, value)
cursor.execute(sql_insert_query, select_tuple) # Compliant, the query is parameterized
connection.commit()
A typical code smell known as unused function parameters refers to parameters declared in a function but not used anywhere within the function’s body. While this might seem harmless at first glance, it can lead to confusion and potential errors in your code. Disregarding the values passed to such parameters, the function’s behavior will be the same, but the programmer’s intention won’t be clearly expressed anymore. Therefore, removing function parameters that are not being utilized is considered best practice.
class C(B):
def do_something(self, a, b): # no issue reported on b
return self.compute(a)
When placing Unicode Grapheme Clusters (characters which require to be encoded in multiple Code Points) inside a character class of a regular expression, this will likely lead to unintended behavior.
For instance, the grapheme cluster c̈ requires two code points: one for ‘c’, followed by one for the umlaut modifier . If placed within a character class, such as [c̈], the regex will consider the character class being the enumeration instead. It will, therefore, match every ‘c’ and every umlaut that isn’t expressed as a single codepoint, which is extremely unlikely to be the intended behavior.
This rule raises an issue every time Unicode Grapheme Clusters are used within a character class of a regular expression.
re.sub(r"[c̈d̈]", "X", "cc̈d̈d") # Noncompliant, print "XXXXXX" instead of expected "cXXd".
Shared naming conventions allow teams to collaborate efficiently.
This rule raises an issue when a class name does not match a provided regular expression.
class myClass: # Noncompliant
...
class my_CONTEXT_manager: # Noncompliant
def __enter__(self):
pass
def __exit__(self, type, value, traceback):
pass
A regex should never include a repetitive pattern whose body would match the empty string. This is usually a sign that a part of the regex is redundant or does not do what the author intended.
r"(?:)*" # same as the empty regex, the '*' accomplishes nothing
r"(?:|x)*" # same as the empty regex, the alternative has no effect
r"(?:x|)*" # same as 'x*', the empty alternative has no effect
r"(?:x*|y*)*" # same as 'x*', the first alternative would always match, y* is never tried
r"(?:x?)*" # same as 'x*'
r"(?:x?)+" # same as 'x*'
Constructing arguments of system commands from user input is security-sensitive. It has led in the past to the following vulnerabilities:
Arguments of system commands are processed by the executed program. The arguments are usually used to configure and influence the behavior of the programs. Control over a single argument might be enough for an attacker to trigger dangerous features like executing arbitrary commands or writing files into specific directories.
import subprocess
input = request.get('input')
if input in allowed:
subprocess.run(["/usr/bin/find", input])
Signaling processes or process groups can seriously affect the stability of this application or other applications on the same system.
Accidentally setting an incorrect PID or signal or allowing untrusted sources to assign arbitrary values to these parameters may result in a denial of service.
Also, the system treats the signal differently if the destination PID is less than or equal to 0. This different behavior may affect multiple processes with the same (E)UID simultaneously if the call is left uncontrolled.
import os
@app.route("/kill-pid/<pid>")
def send_signal(pid):
os.kill(pid, 9) # Sensitive
@app.route("/kill-pgid/<pgid>")
def send_signal(pgid):
os.killpg(pgid, 9) # Sensitive
There is no reason to re-assign a variable to itself. Either this statement is redundant and should be removed, or the re-assignment is a mistake and some other value or variable was intended for the assignment instead.
name = name
To reduce the risk of cross-site scripting attacks, templating systems, such as `Twig, Django, Smarty, Groovy’s template engine, allow configuration of automatic variable escaping before rendering templates. When escape occurs, characters that make sense to the browser (eg: <a>) will be transformed/replaced with escaped/sanitized values (eg: & lt;a& gt; ).
Auto-escaping is not a magic feature to annihilate all cross-site scripting attacks, it depends on the strategy applied and the context, for example a “html auto-escaping” strategy (which only transforms html characters into html entities) will not be relevant when variables are used in a html attribute because ’:’ character is not escaped and thus an attack as below is possible:
<a href="{{ myLink }}">link</a> // myLink = javascript:alert(document.cookie) <a href="javascript:alert(document.cookie)">link</a> // JS injection (XSS attack)
from jinja2 import Environment
env = Environment(autoescape=True) # Compliant
Multiple spaces in a regular expression can make it hard to tell how many spaces should be matched. It’s more readable to use only one space and then indicate with a quantifier how many spaces are expected.
r"Hello, world!"
User-provided data, such as URL parameters, POST data payloads, or cookies, should always be considered untrusted and tainted. Applications constructing HTTP response headers based on tainted data could allow attackers to change security sensitive headers like Cross-Origin Resource Sharing headers.
Web application frameworks and servers might also allow attackers to inject new line characters in headers to craft malformed HTTP response. In this case the application would be vulnerable to a larger range of attacks like HTTP Response Splitting/Smuggling. Most of the time this type of attack is mitigated by default modern web application frameworks but there might be rare cases where older versions are still vulnerable.
As a best practice, applications that use user-provided data to construct the response header should always validate the data first. Validation should be based on a whitelist.
from flask import Response, request
from werkzeug.datastructures import Headers
@app.route('/route')
def route():
content_type = request.args["Content-Type"]
response = Response()
headers = Headers()
headers.add("Content-Type", content_type) # Noncompliant
response.headers = headers
return response
Amazon Elastic Block Store (EBS) is a block-storage service for Amazon Elastic Compute Cloud (EC2). EBS volumes can be encrypted, ensuring the security of both data-at-rest and data-in-transit between an instance and its attached EBS storage. In the case that adversaries gain physical access to the storage medium they are not able to access the data. Encryption can be enabled for specific volumes or for all new volumes and snapshots. Volumes created from snapshots inherit their encryption configuration. A volume created from an encrypted snapshot will also be encrypted by default.
from aws_cdk.aws_ec2 import Volume
class EBSVolumeStack(Stack):
def __init__(self, scope: Construct, construct_id: str, **kwargs) None:
super().__init__(scope, construct_id, **kwargs)
Volume(self,
"unencrypted-explicit",
availability_zone="eu-west-1a",
size=Size.gibibytes(1),
encrypted=False # Sensitive
)
When a cookie is protected with the secure attribute set to true it will not be send by the browser over an unencrypted HTTP request and thus cannot be observed by an unauthorized person during a man-in-the-middle attack.
from flask import Response
@app.route('/')
def index():
response = Response()
response.set_cookie('key', 'value', secure=True) # Compliant
return response
Amazon Elastic File System (EFS) is a serverless file system that does not require provisioning or managing storage. Stored files can be automatically encrypted by the service. In the case that adversaries gain physical access to the storage medium or otherwise leak a message they are not able to access the data.
from aws_cdk import (
aws_efs as efs
)
efs.FileSystem(
self,
"example",
encrypted=False # Sensitive
)
Having too many return statements in a function increases the function’s essential complexity because the flow of execution is broken each time a return statement is encountered. This makes it harder to read and understand the logic of the function.
def fun(): # Noncompliant as there are 4 return statements
if condition1:
return True
elif condition2:
return False
else:
return True
return False
When writing code, it is important to ensure that each statement serves a purpose and contributes to the overall functionality of the program. When they have no side effects or do not change the control flow, they can either indicate a programming error or be redundant:
The code does not behave as intended: The statements are expected to have an effect but they do not. This can be caused by mistyping, copy-and-paste errors, etc.
The statements are residual after a refactoring.
def foo():
bar()
"""Some comment""" # Compliant by default. Noncompliant with "reportOnStrings" set to "true"
qix()
Creating APIs without authentication unnecessarily increases the attack surface on the target infrastructure.
Unless another authentication method is used, attackers have the opportunity to attempt attacks against the underlying API. This means attacks both on the functionality provided by the API and its infrastructure.
from aws_cdk import (
aws_apigateway as apigateway
)
resource = api.root.add_resource("example")
resource.add_method(
"GET",
authorization_type=apigateway.AuthorizationType.NONE # Sensitive
)
Without proper validation or sanitation, an attacker with control over a header’s content could insert line return characters in it and inject arbitrary content in the HTTP response. This includes headers, their content, and the response body.
from http.server import BaseHTTPRequestHandler
from urllib.parse import urlparse, parse_qs
class ReqHandler(BaseHTTPRequestHandler):
def do_GET(self):
parsed = urlparse(self.path)
params = parse_qs(parsed.query)
self.send_response(200)
self.send_header("Content-Type", params.get('accept')[0]) # Noncompliant
self.end_headers()
self.wfile.write(bytes("Hello World!", "utf-8"))
Duplicated string literals make the process of refactoring complex and error-prone, as any change would need to be propagated on all occurrences.
def run():
prepare("action1") # Noncompliant - "action1" is duplicated 3 times
execute("action1")
release("action1")
@app.route("/api/users/", methods=['GET', 'POST', 'PUT'])
def users():
pass
@app.route("/api/projects/", methods=['GET', 'POST', 'PUT']) # Compliant - strings inside decorators are ignored
def projects():
pass
According to the US National Institute of Standards and Technology (NIST), the Data Encryption Standard (DES) is no longer considered secure:
For similar reasons, RC2 should also be avoided.
from pyDes import *
data = "content to be encrypted"
k = des("DESCRYPT", CBC, "\0\0\0\0\0\0\0\0", pad=None, padmode=PAD_PKCS5) // Noncompliant
d = k.encrypt(data)
Predefined permissions, also known as canned ACLs, are an easy way to grant large privileges to predefined groups or users.
The following canned ACLs are security-sensitive:
`PublicRead, PublicReadWrite grant respectively “read” and “read and write” privileges to everyone in the world (AllUsers group).
AuthenticatedRead grants “read” privilege to all authenticated users (AuthenticatedUsers` group).
bucket = s3.Bucket(self, "bucket",
access_control=s3.BucketAccessControl.PUBLIC_READ_WRITE # Sensitive
)
s3deploy.BucketDeployment(self, "DeployWebsite",
access_control=s3.BucketAccessControl.PUBLIC_READ_WRITE # Sensitive
)
The MD5 algorithm and its successor, SHA-1, are no longer considered secure, because it is too easy to create hash collisions with them. That is, it takes too little computational effort to come up with a different input that produces the same MD5 or SHA-1 hash, and using the new, same-hash value gives an attacker the same access as if he had the originally-hashed value. This applies as well to the other Message-Digest algorithms: MD2, MD4, MD6, HAVAL-128, HMAC-MD5, DSA (which uses SHA-1), RIPEMD, RIPEMD-128, RIPEMD-160, HMACRIPEMD160.
Consider using safer alternatives, such as SHA-256, SHA-512 or SHA-3.
import hashlib
m = hashlib.md5() // Noncompliant
It’s confusing to have a class member with the same name (case differences aside) as its enclosing class. This is particularly so when you consider the common practice of naming a class instance for the class itself.
Best practice dictates that any field or member with the same name as the enclosing class be renamed to be more descriptive of the particular aspect of the class it represents or holds.
class Foo:
foo = ''
def getFoo(self):
...
foo = Foo()
foo.getFoo() # what does this return?
Shared coding conventions allow teams to collaborate efficiently. This rule checks that all function names match a provided regular expression.
def MyFunction(a,b):
...
Two s having the same implementation are suspicious. It might be that something else was intended. Or the duplication is intentional, which becomes a maintenance burden.
class MyClass:
code = "secret"
def calculate_code(self):
self.do_the_thing()
return self.__class__.code
def get_name(self): # Noncompliant: duplicates calculate_code
self.do_the_thing()
return self.__class__.code
def do_the_thing(self):
pass # on purpose
Hardcoding IP addresses is security-sensitive. It has led in the past to the following vulnerabilities:
Today’s services have an ever-changing architecture due to their scaling and redundancy needs. It is a mistake to think that a service will always have the same IP address. When it does change, the hardcoded IP will have to be modified too. This will have an impact on the product development, delivery, and deployment:
The developers will have to do a rapid fix every time this happens, instead of having an operation team change a configuration file.
It misleads to use the same address in every environment (dev, sys, qa, prod).
Last but not least it has an effect on application security. Attackers might be able to decompile the code and thereby discover a potentially sensitive address. They can perform a Denial of Service attack on the service, try to get access to the system, or try to spoof the IP address to bypass security checks. Such attacks can always be possible, but in the case of a hardcoded IP address solving the issue will take more time, which will increase an attack’s impact.
ip = config.get(section, ipAddress)
sock = socket.socket()
sock.bind((ip, 9090))
A cookie’s domain specifies which websites should be able to read it. Left blank, browsers are supposed to only send the cookie to sites that exactly match the sending domain. For example, if a cookie was set by lovely.dream.com, it should only be readable by that domain, and not by nightmare.com or even strange.dream.com. If you want to allow sub-domain access for a cookie, you can specify it by adding a dot in front of the cookie’s domain, like so: .dream.com. But cookie domains should always use at least two levels.
Cookie domains can be set either programmatically or via configuration. This rule raises an issue when any cookie domain is set with a single level, as in .com.
import http.cookies
setCookie = http.cookies.SimpleCookie()
setCookie['restricted_cookie'] = 'cookie_value'
setCookie['restricted_cookie']['domain'] = '.sonarsource.com' # Compliant
setCookie['restricted_cookie']['path'] = '/'
Developers often use TODO tags to mark areas in the code where additional work or improvements are needed but are not implemented immediately. However, these TODO tags sometimes get overlooked or forgotten, leading to incomplete or unfinished code. This rule aims to identify and address unattended TODO tags to ensure a clean and maintainable codebase. This description explores why this is a problem and how it can be fixed to improve the overall code quality.
def doSomething:
# TODO : Complete function
A policy that allows identities to access all resources in an AWS account may violate the principle of least privilege. Suppose an identity has permission to access all resources even though it only requires access to some non-sensitive ones. In this case, unauthorized access and disclosure of sensitive information will occur.
from aws_cdk.aws_iam import Effect, PolicyDocument, PolicyStatement
PolicyDocument(
statements=[
PolicyStatement(
effect=Effect.ALLOW,
actions="iam:CreatePolicyVersion",
resources=["*"] # Sensitive
)
]
)
Character classes in regular expressions are a convenient way to match one of several possible characters by listing the allowed characters or ranges of characters. If the same character is listed twice in the same character class or if the character class contains overlapping ranges, this has no effect.
Thus duplicate characters in a character class are either a simple oversight or a sign that a range in the character class matches more than is intended or that the author misunderstood how character classes work and wanted to match more than one character. A common example of the latter mistake is trying to use a range like [0-99] to match numbers of up to two digits, when in fact it is equivalent to [0-9]. Another common cause is forgetting to escape the - character, creating an unintended range that overlaps with other characters in the character class.
r"[0-99]" # Noncompliant, this won't actually match strings with two digits
r"[0-9.-_]" # Noncompliant, .-_ is a range that already contains 0-9 (as well as various other characters such as capital letters)
If an alternative in a regular expression only matches things that are already matched by another alternative, that alternative is redundant and serves no purpose.
In the best case this means that the offending subpattern is merely redundant and should be removed. In the worst case it’s a sign that this regex does not match what it was intended to match and should be reworked.
r"[ab]|a" # Noncompliant: the "|a" is redundant because "[ab]" already matches "a"
r".*|a" # Noncompliant: .* matches everything, so any other alternative is redundant
A naming convention in software development is a set of guidelines for naming code elements like variables, functions, and classes.
The goal of a naming convention is to make the code more readable and understandable, which makes it easier to maintain and debug. It also ensures consistency in the code, especially when multiple developers are working on the same project.
This rule checks that field names match a provided regular expression.
class MyClass:
myField = 1
In Unix file system permissions, the “others” category refers to all
users except the owner of the file system resource and the members of the group
assigned to this resource.
Granting permissions to this category can lead to unintended access to files or directories that could allow attackers to obtain sensitive information, disrupt services or elevate privileges.
os.umask(0o777)
Cryptographic hash algorithms such as MD2, MD4, MD5, MD6, HAVAL-128, HMAC-MD5, DSA (which uses SHA-1), RIPEMD, RIPEMD-128, RIPEMD-160, HMACRIPEMD160 and SHA-1 are no longer considered secure, because it is possible to have collisions (little computational effort is enough to find two or more different inputs that produce the same hash).
import hashlib
m = hashlib.sha512() // Compliant
Possessive quantifiers in Regex patterns like below improve performance by eliminating needless backtracking:
?+ , *+ , ++ , n+ , n,+ , n,m+
But because possessive quantifiers do not keep backtracking positions and never give back, the following sub-patterns should not match only similar characters. Otherwise, possessive quantifiers consume all characters that could have matched the following sub-patterns and nothing remains for the following sub-patterns.
import re
pattern1 = re.compile(r"a++abc", re.DOTALL) # Noncompliant, the second 'a' never matches
pattern2 = re.compile(r"\d*+[02468]", re.DOTALL) # Noncompliant, the sub-pattern "[02468]" never matches
Amazon Simple Queue Service (SQS) is a managed message queuing service for application-to-application (A2A) communication. Amazon SQS can store messages encrypted as soon as they are received. In the case that adversaries gain physical access to the storage medium or otherwise leak a message from the file system, for example through a vulnerability in the service, they are not able to access the data.
from aws_cdk import (
aws_sqs as sqs
)
class QueueStack(Stack):
def __init__(self, scope: Construct, construct_id: str, **kwargs) None:
super().__init__(scope, construct_id, **kwargs)
sqs.Queue( # Sensitive, unencrypted by default
self,
"example"
)
By default S3 buckets are private, it means that only the bucket owner can access it.
This access control can be relaxed with ACLs or policies.
To prevent permissive policies to be set on a S3 bucket the following settings can be configured:
`BlockPublicAcls: to block or not public ACLs to be set to the S3 bucket.
IgnorePublicAcls: to consider or not existing public ACLs set to the S3 bucket.
BlockPublicPolicy: to block or not public policies to be set to the S3 bucket.
RestrictPublicBuckets: to restrict or not the access to the S3 endpoints of public policies to the principals within the bucket owner account.
bucket = s3.Bucket(self,
"bucket" # Sensitive
)
There are several reasons to use a group in a regular expression:
to change the precedence (e.g. do(g|or) will match ‘dog’ and ‘door’)
to remember parenthesised part of the match in the case of capturing group
to improve readability
In any case, having an empty group is most probably a mistake. Either it is a leftover after refactoring and should be removed, or the actual parentheses were intended and were not escaped.
r"foo()" # Noncompliant, will match only 'foo'
Because it is easy to extract strings from an application source code or binary, credentials should not be hard-coded. This is particularly true for applications that are distributed or that are open-source.
In the past, it has led to the following vulnerabilities:
Credentials should be stored outside of the code in a configuration file, a database, or a management service for secrets.
This rule flags instances of hard-coded credentials used in database and LDAP connections. It looks for hard-coded credentials in connection strings, and for variable names that match any of the patterns from the provided list.
It’s recommended to customize the configuration of this rule with additional credential words such as “oauthToken”, “secret”, …
import os
username = os.getenv("username") # Compliant
password = os.getenv("password") # Compliant
usernamePassword = 'user=%s&password=%s' % (username, password) # Compliant{code}
TODO and FIXME comments are typically intended to be short-lived; they are placeholders and reminders that programmers leave for themselves. Unfortunately, even with the best of intentions, those comments are not always acted on and removed in a timely manner. Thus, they can become mysterious, lingering cruft in a code base, reducing both readability and understand-ability.
This rule flags all FIXME and TODO comments that do not have an attribution matching the specified regular expression immediately after the FIXME or TODO. Ideally, such comments will also contain information about what needs to be fixed or done, but this rule does not enforce that.
# TODO
Cloud platforms such as AWS, Azure, or GCP support virtual firewalls that can be used to restrict access to services by controlling inbound and outbound traffic. Any firewall rule allowing traffic from all IP addresses to standard network ports on which administration services traditionally listen, such as 22 for SSH, can expose these services to exploits and unauthorized access.
from aws_cdk import aws_ec2 as ec2
instance = ec2.Instance(
self,
"my_instance",
instance_type=nano_t2,
machine_image=ec2.MachineImage.latest_amazon_linux(),
vpc=vpc
)
instance.connections.allow_from(
ec2.Peer.any_ipv4(), # Noncompliant
ec2.Port.tcp(22),
description="Allows SSH from all IPv4"
)
instance.connections.allow_from_any_ipv4( # Noncompliant
ec2.Port.tcp(3389),
description="Allows Terminal Server from all IPv4"
)
An unused local variable is a variable that has been declared but is not used anywhere in the block of code where it is defined. It is dead code, contributing to unnecessary complexity and leading to confusion when reading the code. Therefore, it should be removed from your code to maintain clarity and efficiency.
for _ in range(10):
do_something()
username, login, _ = auth
do_something_else(username, login)
Loop boundary injections occur in an application when the application retrieves data from a user or a third-party service and inserts it into a loop or a function acting as a loop, without sanitizing it first.
If an application contains a loop that is vulnerable to injections, it is exposed to attacks that target its availability where that loop is used.
A user with malicious intent carefully performs actions whose goal is to cause the loop to run for more iterations than the developer intended, resulting in unexpected behavior or even a crash of the program.
After creating the malicious request, the attacker can attack the servers affected by this vulnerability without relying on any prerequisites.
def example():
limit = int(request.args.get('limit'))
for _ in range(limit):
# ...
Dead stores refer to assignments made to local variables that are subsequently never used or immediately overwritten. Such assignments are unnecessary and don’t contribute to the functionality or clarity of the code. They may even negatively impact performance. Removing them enhances code cleanliness and readability. Even if the unnecessary operations do not do any harm in terms of the program’s correctness, they are - at best - a waste of computing resources.
def func(a, b, compute):
i = a + b # Noncompliant; calculation result not used before value is overwritten
i = compute()
return i
Cognitive Complexity is a measure of how hard it is to understand the control flow of a unit of code. Code with high cognitive complexity is hard to read, understand, test, and modify.
As a rule of thumb, high cognitive complexity is a sign that the code should be refactored into smaller, easier-to-manage pieces.
def process_eligible_users(users):
for user in users: # +1 (for)
if ((user.is_active and # +1 (if) +1 (nested) +1 (multiple conditions)
user.has_profile) or # +1 (mixed operator)
user.age > 18 ):
user.process()
Control flow constructs like if-statements allow the programmer to direct the flow of a program depending on a boolean expression. However, if the condition is always true or always false, only one of the branches will ever be executed. In that case, the control flow construct and the condition no longer serve a purpose; they become gratuitous.
def f(b):
a = True
if a: # Noncompliant
do_something()
if a and b: # Noncompliant; "a" is always "True"
do_something_else()
This rule verifies that single-line comments are not located at the ends of lines of code. The main idea behind this rule is that in order to be really readable, trailing comments would have to be properly written and formatted (correct alignment, no interference with the visual structure of the code, not too long to be visible) but most often, automatic code formatters would not handle this correctly: the code would end up less readable. Comments are far better placed on the previous empty line of code, where they will always be visible and properly formatted.