Skip to content

Commit

Permalink
Update rule metadata in preparation for release 4.6 (#1529)
Browse files Browse the repository at this point in the history
  • Loading branch information
@anton-haubner-sonarsource
anton-haubner-sonarsource authored Aug 3, 2023
1 parent c92dbda commit 65112e8
Show file tree
Hide file tree
Showing 10 changed files with 142 additions and 84 deletions.
5 changes: 2 additions & 3 deletions python-checks/src/main/resources/org/sonar/l10n/py/rules/python/S104.html
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
<h2>Why is this an issue?</h2>
<p>A source file that grows too much tends to aggregate too many responsibilities and inevitably becomes harder to understand and, therefore, to
maintain.</p>
<p>When a source file grows too much, it can accumulate numerous responsibilities and become challenging to understand and maintain.</p>
<p>Above a specific threshold, refactor the file into smaller files whose code focuses on well-defined tasks. Those smaller files will be easier to
understand and easier to test.</p>
understand and test.</p>

74 changes: 39 additions & 35 deletions python-checks/src/main/resources/org/sonar/l10n/py/rules/python/S2053.html
View file
Original file line number Diff line number Diff line change
@@ -1,48 +1,52 @@
<p>This vulnerability increases the likelihood that attackers are able to compute the cleartext of password hashes.</p>
<h2>Why is this an issue?</h2>
<p>In cryptography, a "salt" is an extra piece of data which is included when hashing a password. This makes <code>rainbow-table attacks</code> more
difficult. Using a cryptographic hash function without an unpredictable salt increases the likelihood that an attacker could successfully find the
hash value in databases of precomputed hashes (called <code>rainbow-tables</code>).</p>
<p>This rule raises an issue when a hashing function which has been specifically designed for hashing passwords, such as <code>PBKDF2</code>, is used
with a non-random, reused or too short salt value. It does not raise an issue on base hashing algorithms such as <code>sha1</code> or <code>md5</code>
as they should not be used to hash passwords.</p>
<h2>Recommended Secure Coding Practices</h2>
<ul>
<li> Use hashing functions generating their own secure salt or generate a secure random value of at least 16 bytes. </li>
<li> The salt should be unique by user password. </li>
</ul>
<h3>Noncompliant code example</h3>
<p>hashlib</p>
<pre>
<p>During the process of password hashing, an additional component, known as a "salt," is often integrated to bolster the overall security. This salt,
acting as a defensive measure, primarily wards off certain types of attacks that leverage pre-computed tables to crack passwords.</p>
<p>However, potential risks emerge when the salt is deemed insecure. This can occur when the salt is consistently the same across all users or when it
is too short or predictable. In scenarios where users share the same password and salt, their password hashes will inevitably mirror each other.
Similarly, a short salt heightens the probability of multiple users unintentionally having identical salts, which can potentially lead to identical
password hashes. These identical hashes streamline the process for potential attackers to recover clear-text passwords. Thus, the emphasis on
implementing secure, unique, and sufficiently lengthy salts in password-hashing functions is vital.</p>
<h3>What is the potential impact?</h3>
<p>Despite best efforts, even well-guarded systems might have vulnerabilities that could allow an attacker to gain access to the hashed passwords.
This could be due to software vulnerabilities, insider threats, or even successful phishing attempts that give attackers the access they need.</p>
<p>Once the attacker has these hashes, they will likely attempt to crack them using a couple of methods. One is brute force, which entails trying
every possible combination until the correct password is found. While this can be time-consuming, having the same salt for all users or a short salt
can make the task significantly easier and faster.</p>
<p>If multiple users have the same password and the same salt, their password hashes would be identical. This means that if an attacker successfully
cracks one hash, they have effectively cracked all identical ones, granting them access to multiple accounts at once.</p>
<p>A short salt, while less critical than a shared one, still increases the odds of different users having the same salt. This might create clusters
of password hashes with identical salt that can then be attacked as explained before.</p>
<p>With short salts, the probability of a collision between two users' passwords and salts couple might be low depending on the salt size. The shorter
the salt, the higher the collision probability. In any case, using longer, cryptographically secure salt should be preferred.</p>
<h2>How to fix it in Python Standard Library</h2>
<h3>Code examples</h3>
<p>The following code contains examples of hard-coded salts.</p>
<h4>Noncompliant code example</h4>
<pre data-diff-id="1" data-diff-type="noncompliant">
import crypt
from hashlib import pbkdf2_hmac

hash = pbkdf2_hmac('sha256', password, b'D8VxSmTZt2E2YV454mkqAY5e', 100000) # Noncompliant: salt is hardcoded
hash = crypt.crypt(password) # Noncompliant
</pre>
<p>crypt</p>
<pre>
hash = crypt.crypt(password) # Noncompliant: salt is not provided
</pre>
<h3>Compliant solution</h3>
<p>hashlib</p>
<pre>
<h4>Compliant solution</h4>
<pre data-diff-id="1" data-diff-type="compliant">
import crypt
from hashlib import pbkdf2_hmac

salt = os.urandom(32)
hash = pbkdf2_hmac('sha256', password, salt, 100000) # Compliant
</pre>
<p>crypt</p>
<pre>
salt = crypt.mksalt(crypt.METHOD_SHA256)
hash = crypt.crypt(password, salt) # Compliant
hash = crypt.crypt(password, salt)
</pre>
<h3>How does this work?</h3>
<p>This code ensures that each user’s password has a unique salt value associated with it. It generates a salt randomly and with a length that
provides the required security level. It uses a salt length of at least 16 bytes (128 bits), as recommended by industry standards.</p>
<p>Here, the compliant code example ensures the salt is random and has a sufficient length by calling the <code>crypt.mksalt</code> function. This one
internally uses a cryptographically secure pseudo random number generator.</p>
<h2>Resources</h2>
<h3>Standards</h3>
<ul>
<li> <a href="https://owasp.org/Top10/A02_2021-Cryptographic_Failures/">OWASP Top 10 2021 Category A2</a> - Cryptographic Failures </li>
<li> <a href="https://www.owasp.org/www-project-top-ten/2017/A3_2017-Sensitive_Data_Exposure">OWASP Top 10 2017 Category A3</a> - Sensitive Data
<li> <a href="https://owasp.org/Top10/A02_2021-Cryptographic_Failures/">OWASP</a> Top 10:2021 A02:2021 - Cryptographic Failures </li>
<li> <a href="https://www.owasp.org/www-project-top-ten/2017/A3_2017-Sensitive_Data_Exposure">OWASP</a> - Top 10 2017 - A03:2017 - Sensitive Data
Exposure </li>
<li> <a href="https://cwe.mitre.org/data/definitions/759">MITRE, CWE-759</a> - Use of a One-Way Hash without a Salt </li>
<li> <a href="https://cwe.mitre.org/data/definitions/760">MITRE, CWE-760</a> - Use of a One-Way Hash with a Predictable Salt </li>
<li> <a href="https://www.sans.org/top25-software-errors/#cat3">SANS Top 25</a> - Porous Defenses </li>
<li> <a href="https://cwe.mitre.org/data/definitions/759">CWE</a> - CWE-759: Use of a One-Way Hash without a Salt </li>
<li> <a href="https://cwe.mitre.org/data/definitions/760">CWE</a> - CWE-760: Use of a One-Way Hash with a Predictable Salt </li>
</ul>

5 changes: 3 additions & 2 deletions python-checks/src/main/resources/org/sonar/l10n/py/rules/python/S4143.html
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<h2>Why is this an issue?</h2>
<p>Storing a value inside a collection at a given key or index and then unconditionally overwriting it without reading the initial value is a case of
"dead store".</p>
a "dead store".</p>
<pre>
def swap(mylist, index1, index2):
tmp = mylist[index2]
Expand All @@ -15,5 +15,6 @@ <h2>Why is this an issue?</h2>
mymap['a']['b'] = 42
mymap['a']['b'] = 42 # Noncompliant
</pre>
<p>At best it is redundant and will confuse the reader, most often it is an error and not what the developer intended to do.</p>
<p>This practice is redundant and will cause confusion for the reader. More importantly, it is often an error and not what the developer intended to
do.</p>

85 changes: 64 additions & 21 deletions python-checks/src/main/resources/org/sonar/l10n/py/rules/python/S5445.html
View file
Original file line number Diff line number Diff line change
@@ -1,36 +1,79 @@
<p>Temporary files are considered insecurely created when the file existence check is performed separately from the actual file creation. Such a
situation can occur when creating temporary files using normal file handling functions or when using dedicated temporary file handling functions that
are not atomic.</p>
<h2>Why is this an issue?</h2>
<p>Creating temporary files using insecure methods exposes the application 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 run with elevated permissions.</p>
<p>In the past, it has led to the following vulnerabilities:</p>
<ul>
<li> <a href="https://nvd.nist.gov/vuln/detail/CVE-2014-1858">CVE-2014-1858</a> </li>
<li> <a href="https://nvd.nist.gov/vuln/detail/CVE-2014-1932">CVE-2014-1932</a> </li>
</ul>
<h3>Noncompliant code example</h3>
<pre>
<p>Creating temporary files in a non-atomic way introduces race condition issues in the application’s behavior. Indeed, a third party can create a
given file between when the application chooses its name and when it creates it.</p>
<p>In such a situation, the application might use a temporary file that it does not entirely control. In particular, this file’s permissions might be
different than expected. This can lead to trust boundary issues.</p>
<h3>What is the potential impact?</h3>
<p>Attackers with control over a temporary file used by a vulnerable application will be able to modify it in a way that will affect the application’s
logic. By changing this file’s Access Control List or other operating system-level properties, they could prevent the file from being deleted or
emptied. They may also alter the file’s content before or while the application uses it.</p>
<p>Depending on why and how the affected temporary files are used, the exploitation of a race condition in an application can have various
consequences. They can range from sensitive information disclosure to more serious application or hosting infrastructure compromise.</p>
<h4>Information disclosure</h4>
<p>Because attackers can control the permissions set on temporary files and prevent their removal, they can read what the application stores in them.
This might be especially critical if this information is sensitive.</p>
<p>For example, an application might use temporary files to store users' session-related information. In such a case, attackers controlling those
files can access session-stored information. This might allow them to take over authenticated users' identities and entitlements.</p>
<h4>Attack surface extension</h4>
<p>An application might use temporary files to store technical data for further reuse or as a communication channel between multiple components. In
that case, it might consider those files part of the trust boundaries and use their content without additional security validation or sanitation. In
such a case, an attacker controlling the file content might use it as an attack vector for further compromise.</p>
<p>For example, an application might store serialized data in temporary files for later use. In such a case, attackers controlling those files'
content can change it in a way that will lead to an insecure deserialization exploitation. It might allow them to execute arbitrary code on the
application hosting server and take it over.</p>
<h2>How to fix it</h2>
<h3>Code examples</h3>
<p>The following code example is vulnerable to a race condition attack because it creates a temporary file using an unsafe API function.</p>
<h4>Noncompliant code example</h4>
<pre data-diff-id="1" data-diff-type="noncompliant">
import tempfile

filename = tempfile.mktemp() # Noncompliant
tmp_file = open(filename, "w+")
</pre>
<h3>Compliant solution</h3>
<pre>
<h4>Compliant solution</h4>
<pre data-diff-id="1" data-diff-type="compliant">
import tempfile

tmp_file1 = tempfile.NamedTemporaryFile(delete=False) # Compliant; Easy replacement to tempfile.mktemp()
tmp_file2 = tempfile.NamedTemporaryFile() # Compliant; Created file will be automatically deleted
tmp_file1 = tempfile.NamedTemporaryFile(delete=False)
tmp_file2 = tempfile.NamedTemporaryFile()
</pre>
<h3>How does this work?</h3>
<p>Applications should create temporary files so that no third party can read or modify their content. It requires that the files' name, location, and
permissions are carefully chosen and set. This can be achieved in multiple ways depending on the applications' technology stacks.</p>
<h4>Use a secure API function</h4>
<p>Temporary files handling APIs generally provide secure functions to create temporary files. In most cases, they operate in an atomical way,
creating and opening a file with a unique and unpredictable name in a single call. Those functions can often be used to replace less secure
alternatives without requiring important development efforts.</p>
<p>Here, the example compliant code uses the more secure <code>tempfile.NamedTemporaryFile</code> function to handle the temporary file creation.</p>
<h4>Strong security controls</h4>
<p>Temporary files can be created using unsafe functions and API as long as strong security controls are applied. Non-temporary file-handling
functions and APIs can also be used for that purpose.</p>
<p>In general, applications should ensure that attackers can not create a file before them. This turns into the following requirements when creating
the files:</p>
<ul>
<li> Files should be created in a non-public directory. </li>
<li> File names should be unique. </li>
<li> File names should be unpredictable. They should be generated using a cryptographically secure random generator. </li>
<li> File creation should fail if a target file already exists. </li>
</ul>
<p>Moreover, when possible, it is recommended that applications destroy temporary files after they have finished using them.</p>
<h2>Resources</h2>
<h3>Documentation</h3>
<ul>
<li> <a href="https://owasp.org/www-community/vulnerabilities/Insecure_Temporary_File">OWASP</a> - Insecure Temporary File </li>
<li> <a href="https://docs.python.org/3/library/tempfile.html#deprecated-functions-and-variables">Python documentation</a> - tempfile </li>
</ul>
<h3>Standards</h3>
<ul>
<li> <a href="https://owasp.org/Top10/A01_2021-Broken_Access_Control/">OWASP Top 10 2021 Category A1</a> - Broken Access Control </li>
<li> <a href="https://owasp.org/www-project-top-ten/2017/A9_2017-Using_Components_with_Known_Vulnerabilities">OWASP Top 10 2017 Category A9</a> -
<li> <a href="https://owasp.org/Top10/A01_2021-Broken_Access_Control/">OWASP</a> - Top 10 2021 - A01:2021 - Broken Access Control </li>
<li> <a href="https://owasp.org/www-project-top-ten/2017/A9_2017-Using_Components_with_Known_Vulnerabilities">OWASP</a> - Top 10 2017 - A9:2017 -
Using Components with Known Vulnerabilities </li>
<li> <a href="https://cwe.mitre.org/data/definitions/377">MITRE, CWE-377</a> - Insecure Temporary File </li>
<li> <a href="https://cwe.mitre.org/data/definitions/379">MITRE, CWE-379</a> - Creation of Temporary File in Directory with Incorrect Permissions
<li> <a href="https://cwe.mitre.org/data/definitions/377">MITRE</a> - CWE-377: Insecure Temporary File </li>
<li> <a href="https://cwe.mitre.org/data/definitions/379">MITRE</a> - CWE-379: Creation of Temporary File in Directory with Incorrect Permissions
</li>
<li> <a href="https://owasp.org/www-community/vulnerabilities/Insecure_Temporary_File">OWASP, Insecure Temporary File</a> </li>
<li> <a href="https://docs.python.org/3/library/tempfile.html#deprecated-functions-and-variables">Python tempfile module</a> </li>
<li> <a href="https://docs.python.org/2.7/library/os.html">Python 2.7 os module</a> </li>
</ul>

14 changes: 10 additions & 4 deletions python-checks/src/main/resources/org/sonar/l10n/py/rules/python/S6303.html
View file
Original file line number Diff line number Diff line change
@@ -1,9 +1,15 @@
<p>Using unencrypted RDS DB resources exposes data to unauthorized access to the underlying storage.<br> This includes database data, logs, automatic
backups, read replicas, snapshots, and cluster metadata.</p>
<p>Using unencrypted RDS DB resources exposes data to unauthorized access.<br> This includes database data, logs, automatic backups, read replicas,
snapshots, and cluster metadata.</p>
<p>This situation can occur in a variety of scenarios, such as:</p>
<ul>
<li> a malicious insider working at the cloud provider gains physical access to the storage device and exfiltrates data. </li>
<li> unknown attackers penetrate the cloud provider’s logical infrastructure and systems for extortion. </li>
<li> A malicious insider working at the cloud provider gains physical access to the storage device. </li>
<li> Unknown attackers penetrate the cloud provider’s logical infrastructure and systems. </li>
</ul>
<p>After a successful intrusion, the underlying applications are exposed to:</p>
<ul>
<li> theft of intellectual property and/or personal data </li>
<li> extortion </li>
<li> denial of services and security bypasses via data corruption or deletion </li>
</ul>
<p>AWS-managed encryption at rest reduces this risk with a simple switch.</p>
<h2>Ask Yourself Whether</h2>
Expand Down
21 changes: 13 additions & 8 deletions python-checks/src/main/resources/org/sonar/l10n/py/rules/python/S6333.html
View file
Original file line number Diff line number Diff line change
@@ -1,14 +1,19 @@
<p>A public API, which can be requested by any authenticated or unauthenticated identities, can lead to unauthorized actions and information
disclosures.</p>
<p>Creating APIs without authentication unnecessarily increases the attack surface on the target infrastructure.</p>
<p>Unless another authentication method is used, attackers have the opportunity to attempt attacks against the underlying API.<br> This means attacks
both on the functionality provided by the API and its infrastructure.</p>
<h2>Ask Yourself Whether</h2>
<p>The public API:</p>
<ul>
<li> exposes sensitive data like personal information. </li>
<li> can be used to perform sensitive operations. </li>
<li> The underlying API exposes all of its contents to any anonymous Internet user. </li>
</ul>
<p>There is a risk if you answered yes to any of those questions.</p>
<p>There is a risk if you answered yes to this question.</p>
<h2>Recommended Secure Coding Practices</h2>
<p>It’s recommended to restrict API access to authorized entities, unless the API offers a non-sensitive service designed to be public.</p>
<p>In general, prefer limiting API access to a specific set of people or entities.</p>
<p>AWS provides multiple methods to do so:</p>
<ul>
<li> <code>AWS_IAM</code>, to use standard AWS IAM roles and policies. </li>
<li> <code>COGNITO_USER_POOLS</code>, to use customizable OpenID Connect (OIDC) identity providers (IdP). </li>
<li> <code>CUSTOM</code>, to use an AWS-independant OIDC provider, glued to the infrastructure with a Lambda authorizer. </li>
</ul>
<h2>Sensitive Code Example</h2>
<p>For <a href="https://docs.aws.amazon.com/cdk/api/v2/python/aws_cdk.aws_apigateway/Resource.html">aws_cdk.aws_apigateway.Resource</a>:</p>
<pre>
Expand Down Expand Up @@ -74,9 +79,9 @@ <h2>Compliant Solution</h2>
</pre>
<h2>See</h2>
<ul>
<li> <a href="https://owasp.org/Top10/A01_2021-Broken_Access_Control/">OWASP Top 10 2021 Category A1</a> - Broken Access Control </li>
<li> <a href="https://docs.aws.amazon.com/apigateway/latest/developerguide/apigateway-control-access-to-api.html">AWS Documentation</a> -
Controlling and managing access to a REST API in API Gateway </li>
<li> <a href="https://owasp.org/Top10/A01_2021-Broken_Access_Control/">OWASP Top 10 2021 Category A1</a> - Broken Access Control </li>
<li> <a href="https://cwe.mitre.org/data/definitions/284">MITRE, CWE-284</a> - Improper Access Control </li>
<li> <a href="https://owasp.org/www-project-top-ten/2017/A5_2017-Broken_Access_Control">OWASP Top 10 2017 Category A5</a> - Broken Access Control
</li>
Expand Down
2 changes: 1 addition & 1 deletion python-checks/src/main/resources/org/sonar/l10n/py/rules/python/S6333.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
"status": "ready",
"remediation": {
"func": "Constant\/Issue",
"constantCost": "5min"
"constantCost": "15min"
},
"tags": [
"aws",
Expand Down
Loading

0 comments on commit 65112e8

Please sign in to comment.