From 65112e84f07950d378d7b3f7db7ffeb732ff0d74 Mon Sep 17 00:00:00 2001 From: Anton Haubner Date: Thu, 3 Aug 2023 11:02:08 +0200 Subject: [PATCH] Update rule metadata in preparation for release 4.6 (#1529) --- .../org/sonar/l10n/py/rules/python/S104.html | 5 +- .../org/sonar/l10n/py/rules/python/S2053.html | 74 ++++++++-------- .../org/sonar/l10n/py/rules/python/S4143.html | 5 +- .../org/sonar/l10n/py/rules/python/S5445.html | 85 ++++++++++++++----- .../org/sonar/l10n/py/rules/python/S6303.html | 14 ++- .../org/sonar/l10n/py/rules/python/S6333.html | 21 +++-- .../org/sonar/l10n/py/rules/python/S6333.json | 2 +- .../org/sonar/l10n/py/rules/python/S6659.html | 16 ++-- .../org/sonar/l10n/py/rules/python/S6659.json | 2 +- sonarpedia.json | 2 +- 10 files changed, 142 insertions(+), 84 deletions(-) diff --git a/python-checks/src/main/resources/org/sonar/l10n/py/rules/python/S104.html b/python-checks/src/main/resources/org/sonar/l10n/py/rules/python/S104.html index cc8ee8fa38..ab59359a87 100644 --- a/python-checks/src/main/resources/org/sonar/l10n/py/rules/python/S104.html +++ b/python-checks/src/main/resources/org/sonar/l10n/py/rules/python/S104.html @@ -1,6 +1,5 @@

Why is this an issue?

-

A source file that grows too much tends to aggregate too many responsibilities and inevitably becomes harder to understand and, therefore, to -maintain.

+

When a source file grows too much, it can accumulate numerous responsibilities and become challenging to understand and maintain.

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.

+understand and test.

diff --git a/python-checks/src/main/resources/org/sonar/l10n/py/rules/python/S2053.html b/python-checks/src/main/resources/org/sonar/l10n/py/rules/python/S2053.html index 08c48284c4..eedd62058d 100644 --- a/python-checks/src/main/resources/org/sonar/l10n/py/rules/python/S2053.html +++ b/python-checks/src/main/resources/org/sonar/l10n/py/rules/python/S2053.html @@ -1,48 +1,52 @@ +

This vulnerability increases the likelihood that attackers are able to compute the cleartext of password hashes.

Why is this an issue?

-

In cryptography, a "salt" is an extra piece of data which is included when hashing a password. This makes rainbow-table attacks 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 rainbow-tables).

-

This rule raises an issue when a hashing function which has been specifically designed for hashing passwords, such as PBKDF2, is used -with a non-random, reused or too short salt value. It does not raise an issue on base hashing algorithms such as sha1 or md5 -as they should not be used to hash passwords.

-

Recommended Secure Coding Practices

- -

Noncompliant code example

-

hashlib

-
+

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.

+

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.

+

What is the potential impact?

+

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.

+

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.

+

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.

+

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.

+

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.

+

How to fix it in Python Standard Library

+

Code examples

+

The following code contains examples of hard-coded salts.

+

Noncompliant code example

+
 import crypt
-from hashlib import pbkdf2_hmac
 
-hash = pbkdf2_hmac('sha256', password, b'D8VxSmTZt2E2YV454mkqAY5e', 100000)    # Noncompliant: salt is hardcoded
+hash = crypt.crypt(password) # Noncompliant
 
-

crypt

-
-hash = crypt.crypt(password)         # Noncompliant: salt is not provided
-
-

Compliant solution

-

hashlib

-
+

Compliant solution

+
 import crypt
-from hashlib import pbkdf2_hmac
 
-salt = os.urandom(32)
-hash = pbkdf2_hmac('sha256', password, salt, 100000)    # Compliant
-
-

crypt

-
 salt = crypt.mksalt(crypt.METHOD_SHA256)
-hash = crypt.crypt(password, salt)         # Compliant
+hash = crypt.crypt(password, salt)
 
+

How does this work?

+

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.

+

Here, the compliant code example ensures the salt is random and has a sufficient length by calling the crypt.mksalt function. This one +internally uses a cryptographically secure pseudo random number generator.

Resources

+

Standards

diff --git a/python-checks/src/main/resources/org/sonar/l10n/py/rules/python/S4143.html b/python-checks/src/main/resources/org/sonar/l10n/py/rules/python/S4143.html index b66463adf3..85daaa5786 100644 --- a/python-checks/src/main/resources/org/sonar/l10n/py/rules/python/S4143.html +++ b/python-checks/src/main/resources/org/sonar/l10n/py/rules/python/S4143.html @@ -1,6 +1,6 @@

Why is this an issue?

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".

+a "dead store".

 def swap(mylist, index1, index2):
     tmp = mylist[index2]
@@ -15,5 +15,6 @@ 

Why is this an issue?

mymap['a']['b'] = 42 mymap['a']['b'] = 42 # Noncompliant
-

At best it is redundant and will confuse the reader, most often it is an error and not what the developer intended to do.

+

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.

diff --git a/python-checks/src/main/resources/org/sonar/l10n/py/rules/python/S5445.html b/python-checks/src/main/resources/org/sonar/l10n/py/rules/python/S5445.html index 60851524b7..6fa9c44a89 100644 --- a/python-checks/src/main/resources/org/sonar/l10n/py/rules/python/S5445.html +++ b/python-checks/src/main/resources/org/sonar/l10n/py/rules/python/S5445.html @@ -1,36 +1,79 @@ +

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.

Why is this an issue?

-

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.

-

In the past, it has led to the following vulnerabilities:

- -

Noncompliant code example

-
+

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.

+

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.

+

What is the potential impact?

+

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.

+

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.

+

Information disclosure

+

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.

+

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.

+

Attack surface extension

+

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.

+

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.

+

How to fix it

+

Code examples

+

The following code example is vulnerable to a race condition attack because it creates a temporary file using an unsafe API function.

+

Noncompliant code example

+
 import tempfile
 
 filename = tempfile.mktemp() # Noncompliant
 tmp_file = open(filename, "w+")
 
-

Compliant solution

-
+

Compliant solution

+
 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()
 
+

How does this work?

+

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.

+

Use a secure API function

+

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.

+

Here, the example compliant code uses the more secure tempfile.NamedTemporaryFile function to handle the temporary file creation.

+

Strong security controls

+

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.

+

In general, applications should ensure that attackers can not create a file before them. This turns into the following requirements when creating +the files:

+
    +
  • Files should be created in a non-public directory.
  • +
  • File names should be unique.
  • +
  • File names should be unpredictable. They should be generated using a cryptographically secure random generator.
  • +
  • File creation should fail if a target file already exists.
  • +
+

Moreover, when possible, it is recommended that applications destroy temporary files after they have finished using them.

Resources

+

Documentation

+ +

Standards

diff --git a/python-checks/src/main/resources/org/sonar/l10n/py/rules/python/S6303.html b/python-checks/src/main/resources/org/sonar/l10n/py/rules/python/S6303.html index b01ea5b9d0..87186b4fbe 100644 --- a/python-checks/src/main/resources/org/sonar/l10n/py/rules/python/S6303.html +++ b/python-checks/src/main/resources/org/sonar/l10n/py/rules/python/S6303.html @@ -1,9 +1,15 @@ -

Using unencrypted RDS DB resources exposes data to unauthorized access to the underlying storage.
This includes database data, logs, automatic -backups, read replicas, snapshots, and cluster metadata.

+

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 and exfiltrates data.
  • -
  • unknown attackers penetrate the cloud provider’s logical infrastructure and systems for extortion.
  • +
  • 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.

Ask Yourself Whether

diff --git a/python-checks/src/main/resources/org/sonar/l10n/py/rules/python/S6333.html b/python-checks/src/main/resources/org/sonar/l10n/py/rules/python/S6333.html index f1993af744..459a227ca4 100644 --- a/python-checks/src/main/resources/org/sonar/l10n/py/rules/python/S6333.html +++ b/python-checks/src/main/resources/org/sonar/l10n/py/rules/python/S6333.html @@ -1,14 +1,19 @@ -

A public API, which can be requested by any authenticated or unauthenticated identities, can lead to unauthorized actions and information -disclosures.

+

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.

Ask Yourself Whether

-

The public API:

    -
  • exposes sensitive data like personal information.
  • -
  • can be used to perform sensitive operations.
  • +
  • The underlying API exposes all of its contents to any anonymous Internet user.
-

There is a risk if you answered yes to any of those questions.

+

There is a risk if you answered yes to this question.

Recommended Secure Coding Practices

-

It’s recommended to restrict API access to authorized entities, unless the API offers a non-sensitive service designed to be public.

+

In general, prefer limiting API access to a specific set of people or entities.

+

AWS provides multiple methods to do so:

+
    +
  • AWS_IAM, to use standard AWS IAM roles and policies.
  • +
  • COGNITO_USER_POOLS, to use customizable OpenID Connect (OIDC) identity providers (IdP).
  • +
  • CUSTOM, to use an AWS-independant OIDC provider, glued to the infrastructure with a Lambda authorizer.
  • +

Sensitive Code Example

For aws_cdk.aws_apigateway.Resource:

@@ -74,9 +79,9 @@ 

Compliant Solution

See

    -
  • OWASP Top 10 2021 Category A1 - Broken Access Control
  • AWS Documentation - Controlling and managing access to a REST API in API Gateway
  • +
  • OWASP Top 10 2021 Category A1 - Broken Access Control
  • MITRE, CWE-284 - Improper Access Control
  • OWASP Top 10 2017 Category A5 - Broken Access Control
  • diff --git a/python-checks/src/main/resources/org/sonar/l10n/py/rules/python/S6333.json b/python-checks/src/main/resources/org/sonar/l10n/py/rules/python/S6333.json index eec757c197..aadbc168e2 100644 --- a/python-checks/src/main/resources/org/sonar/l10n/py/rules/python/S6333.json +++ b/python-checks/src/main/resources/org/sonar/l10n/py/rules/python/S6333.json @@ -4,7 +4,7 @@ "status": "ready", "remediation": { "func": "Constant\/Issue", - "constantCost": "5min" + "constantCost": "15min" }, "tags": [ "aws", diff --git a/python-checks/src/main/resources/org/sonar/l10n/py/rules/python/S6659.html b/python-checks/src/main/resources/org/sonar/l10n/py/rules/python/S6659.html index 275f8e787d..b135c2566d 100644 --- a/python-checks/src/main/resources/org/sonar/l10n/py/rules/python/S6659.html +++ b/python-checks/src/main/resources/org/sonar/l10n/py/rules/python/S6659.html @@ -1,26 +1,26 @@ -

    This rule raises an issue when string slicing is used in condition expressions instead of the startsWith or endsWith +

    This rule raises an issue when string slicing is used in condition expressions instead of the startswith or endswith methods.

    Why is this an issue?

    -

    Using the startsWith and endsWith methods in Python instead of string slicing offers several advantages:

    +

    Using the startswith and endswith methods in Python instead of string slicing offers several advantages:

      -
    1. Readability and Intent: Using startsWith and endsWith methods provides code that is more readable +
    2. Readability and Intent: Using startswith and endswith methods provides code that is more readable and self-explanatory. It clearly communicates your intention to check if a string starts or ends with a specific pattern. This makes the code more maintainable and easier to understand for other developers.
    3. -
    4. Flexibility: The startsWith and endsWith methods allow you to check for patterns of varying lengths. +
    5. Flexibility: The startswith and endswith methods allow you to check for patterns of varying lengths. With string slicing, you would need to specify the exact length of the substring to compare. However, with the methods, you can pass in a pattern of any length, making your code more flexible and adaptable.
    6. Error Handling: The methods handle edge cases automatically. If you pass a substring length that exceeds the length of the original string, slicing would raise an IndexError exception. On the other hand, the methods gracefully handle such cases and return False, avoiding any potential errors.
    7. -
    8. Performance Optimization: In some cases, using startsWith and endsWith methods can provide better +
    9. Performance Optimization: In some cases, using startswith and endswith methods can provide better performance. These methods are optimized and implemented in C, which can make them faster than manually slicing the string in Python. Although the performance gain might be negligible for small strings, it can be significant when working with large strings or processing them in a loop.
    -

    Overall, using startsWith and endsWith methods provides a cleaner, more readable, and error-resistant approach for +

    Overall, using startswith and endswith methods provides a cleaner, more readable, and error-resistant approach for checking if a string starts or ends with a specific pattern. It promotes code clarity, flexibility, and can potentially improve performance. This is also recommended by the PEP8 style guide.

    How to fix it

    -

    Use startsWith and endsWith methods instead of string slicing in condition expressions.

    +

    Use startswith and endswith methods instead of string slicing in condition expressions.

    Code examples

    Noncompliant code example

    @@ -36,7 +36,7 @@ 

    Compliant solution

     message = "Hello, world!"
     
    -if message.startsWith("Hello"):
    +if message.startswith("Hello"):
         ...
     
     if message.endswith("world!"):
    diff --git a/python-checks/src/main/resources/org/sonar/l10n/py/rules/python/S6659.json b/python-checks/src/main/resources/org/sonar/l10n/py/rules/python/S6659.json
    index f191065946..eb59415395 100644
    --- a/python-checks/src/main/resources/org/sonar/l10n/py/rules/python/S6659.json
    +++ b/python-checks/src/main/resources/org/sonar/l10n/py/rules/python/S6659.json
    @@ -1,5 +1,5 @@
     {
    -  "title": "\u0027startsWith\u0027 or \u0027endsWith\u0027 methods should be used instead of string slicing in condition expressions",
    +  "title": "\u0027startswith\u0027 or \u0027endswith\u0027 methods should be used instead of string slicing in condition expressions",
       "type": "CODE_SMELL",
       "status": "ready",
       "remediation": {
    diff --git a/sonarpedia.json b/sonarpedia.json
    index 90bf34a372..97e2aa2426 100644
    --- a/sonarpedia.json
    +++ b/sonarpedia.json
    @@ -3,7 +3,7 @@
       "languages": [
         "PY"
       ],
    -  "latest-update": "2023-07-04T12:26:38.773802Z",
    +  "latest-update": "2023-08-02T16:30:37.327641324Z",
       "options": {
         "no-language-in-filenames": true,
         "preserve-filenames": true