From 7c1b6f5d0d6108b56ca377458e00ac9ef9a72c7e Mon Sep 17 00:00:00 2001 From: Nils Werner <64034005+nils-werner-sonarsource@users.noreply.github.com> Date: Tue, 14 Nov 2023 17:13:30 +0100 Subject: [PATCH] Update rule metadata (#1160) --- .../org/sonar/l10n/php/rules/php/S112.html | 2 +- .../org/sonar/l10n/php/rules/php/S113.html | 2 +- .../org/sonar/l10n/php/rules/php/S113.json | 2 +- .../org/sonar/l10n/php/rules/php/S1603.html | 2 +- .../org/sonar/l10n/php/rules/php/S3333.html | 63 ++++++++++++------- .../org/sonar/l10n/php/rules/php/S3333.json | 1 - .../org/sonar/l10n/php/rules/php/S3415.html | 4 +- sonarpedia.json | 2 +- 8 files changed, 46 insertions(+), 32 deletions(-) diff --git a/php-checks/src/main/resources/org/sonar/l10n/php/rules/php/S112.html b/php-checks/src/main/resources/org/sonar/l10n/php/rules/php/S112.html index f409b38533..30592b1648 100644 --- a/php-checks/src/main/resources/org/sonar/l10n/php/rules/php/S112.html +++ b/php-checks/src/main/resources/org/sonar/l10n/php/rules/php/S112.html @@ -15,7 +15,7 @@

How to fix it

Code examples

Noncompliant code example

diff --git a/php-checks/src/main/resources/org/sonar/l10n/php/rules/php/S113.html b/php-checks/src/main/resources/org/sonar/l10n/php/rules/php/S113.html index 33ec564a16..65ddd8f677 100644 --- a/php-checks/src/main/resources/org/sonar/l10n/php/rules/php/S113.html +++ b/php-checks/src/main/resources/org/sonar/l10n/php/rules/php/S113.html @@ -1,5 +1,5 @@

Why is this an issue?

-

Some tools work better when files end with an empty line.

+

Some tools work better when files end with a newline.

This rule simply generates an issue if it is missing.

For example, a Git diff looks like this if the empty line is missing at the end of the file:

diff --git a/php-checks/src/main/resources/org/sonar/l10n/php/rules/php/S113.json b/php-checks/src/main/resources/org/sonar/l10n/php/rules/php/S113.json
index b5acb1513c..1d0fcfdd81 100644
--- a/php-checks/src/main/resources/org/sonar/l10n/php/rules/php/S113.json
+++ b/php-checks/src/main/resources/org/sonar/l10n/php/rules/php/S113.json
@@ -1,5 +1,5 @@
 {
-  "title": "Files should contain an empty newline at the end",
+  "title": "Files should end with a newline",
   "type": "CODE_SMELL",
   "code": {
     "impacts": {
diff --git a/php-checks/src/main/resources/org/sonar/l10n/php/rules/php/S1603.html b/php-checks/src/main/resources/org/sonar/l10n/php/rules/php/S1603.html
index f86e3e5101..fb0a442002 100644
--- a/php-checks/src/main/resources/org/sonar/l10n/php/rules/php/S1603.html
+++ b/php-checks/src/main/resources/org/sonar/l10n/php/rules/php/S1603.html
@@ -1,7 +1,7 @@
 

Why is this an issue?

Using a function in PHP with the same name as the nesting class was historically used to declare a class constructor. However, as of PHP 8.0.0, this declaration is discouraged and will provoke an E_DEPRECATED error, albeit it functions as a constructor.

-

Instead, users should explicitly define the constructor by declaring a __construct(…​) function. However, if both styles are present +

Instead, users should explicitly define the constructor by declaring a __construct(...) function. However, if both styles are present in the same class, PHP will treat the __construct function as the class constructor, which can cause unintended behavior.

Adhering to this convention improves readability and maintainability by ensuring that the constructor declaration is named uniformly throughout the codebase.

diff --git a/php-checks/src/main/resources/org/sonar/l10n/php/rules/php/S3333.html b/php-checks/src/main/resources/org/sonar/l10n/php/rules/php/S3333.html index 36c0c91fd8..d9101c7b96 100644 --- a/php-checks/src/main/resources/org/sonar/l10n/php/rules/php/S3333.html +++ b/php-checks/src/main/resources/org/sonar/l10n/php/rules/php/S3333.html @@ -1,33 +1,48 @@ +

When accessing files on the local filesystem, PHP can enforce security checks to defend against some attacks. The open_basedir setting +in the main PHP configuration defines a set of directories that the application is allowed to access. Access to locations outside of these directories +will be blocked.

Why is this an issue?

-

The open_basedir configuration in php.ini limits the files the script can access using, for example, include and -fopen(). Leave it out, and there is no default limit, meaning that any file can be accessed. Include it, and PHP will refuse to access -files outside the allowed path.

-

open_basedir should be configured with a directory, which will then be accessible recursively. However, the use of . -(current directory) as an open_basedir value should be avoided since it’s resolved dynamically during script execution, so a -chdir('/') command could lay the whole server open to the script.

-

This is not a fool-proof configuration; it can be reset or overridden at the script level. But its use should be seen as a minimum due diligence -step. This rule raises an issue when open_basedir is not present in php.ini, and when open_basedir contains root, -or the current directory (.) symbol.

-

Noncompliant code example

-
-; php.ini try 1
-; open_basedir="${USER}/scripts/data"  Noncompliant; commented out
-
-; php.ini try 2
+

The PHP runtime will allow the application to access all files underneath the configured set of directories. If no value is set, the application +may access any file on the filesystem.

+

What is the potential impact?

+

open_basedir is commonly used to ensure that a PHP application can only access files needed for the application function. While +deactivating this setting does not pose a direct threat to the application’s security, it can make exploitation of other vulnerabilities easier and +more severe.

+

If an attacker can exploit a path traversal vulnerability, they will be able to access any file made available to the application’s user account. +This may include system-critical or otherwise sensitive files.

+

In shared hosting environments, a vulnerability can affect all co-hosted applications and not only the vulnerable one. open_basedir +can help limit the scope of the compromise in that case.

+

How to fix it

+

The main PHP configuration should define the open_basedir setting. This setting should not include overly large directories, such as +the root directory of the filesystem.

+

Adding the current directory, denoted by “.”, to the open_basedir configuration is also dangerous. It is possible to change the +current directory within PHP scripts by calling chdir(), effectively removing any protection.

+

Code examples

+

Noncompliant code example

+
+; php.ini
 open_basedir="/:${USER}/scripts/data"  ; Noncompliant; root directory in the list
 
-

Compliant solution

-
-; php.ini try 1
+
+; php.ini
+; open_basedir= ; Noncompliant; setting commented out
+
+

Compliant solution

+
+; php.ini
 open_basedir="${USER}/scripts/data"
 
+
+; php.ini try 1
+open_basedir="/var/www/myapp/data"
+

Resources

+

Standards

diff --git a/php-checks/src/main/resources/org/sonar/l10n/php/rules/php/S3333.json b/php-checks/src/main/resources/org/sonar/l10n/php/rules/php/S3333.json index 4ae5363100..7b21f8442b 100644 --- a/php-checks/src/main/resources/org/sonar/l10n/php/rules/php/S3333.json +++ b/php-checks/src/main/resources/org/sonar/l10n/php/rules/php/S3333.json @@ -29,7 +29,6 @@ "A6" ], "OWASP Top 10 2021": [ - "A1", "A5" ], "PCI DSS 3.2": [ diff --git a/php-checks/src/main/resources/org/sonar/l10n/php/rules/php/S3415.html b/php-checks/src/main/resources/org/sonar/l10n/php/rules/php/S3415.html index e182c7173b..f2350858cc 100644 --- a/php-checks/src/main/resources/org/sonar/l10n/php/rules/php/S3415.html +++ b/php-checks/src/main/resources/org/sonar/l10n/php/rules/php/S3415.html @@ -1,6 +1,6 @@

Why is this an issue?

-

The standard PHPUnit assertion methods such as assertEquals, expect the first argument to be the expected value and the -second argument to be the actual value.

+

The standard PHPUnit assertion methods such as __assertEquals__, expect the first argument to be the expected value and the second +argument to be the actual value.

What is the potential impact?

Having the expected value and the actual value in the wrong order will not alter the outcome of tests, (succeed/fail when it should) but the error messages will contain misleading information.

diff --git a/sonarpedia.json b/sonarpedia.json index 1713078a87..b8f4c8eb6b 100644 --- a/sonarpedia.json +++ b/sonarpedia.json @@ -3,7 +3,7 @@ "languages": [ "PHP" ], - "latest-update": "2023-10-25T13:45:51.374777800Z", + "latest-update": "2023-11-14T14:50:12.166348Z", "options": { "no-language-in-filenames": true, "preserve-filenames": true