Skip to content

Commit

Permalink
SONARPHP-939: Deprecate S2278 in favor of S5547 (#577)
Browse files Browse the repository at this point in the history
* rules api update

* deprecate S2277 and S2278

* deprecate S2277 and S2278
  • Loading branch information
karim-ouerghemmi-sonarsource authored Jul 3, 2020
1 parent a0ff488 commit dd01918
Show file tree
Hide file tree
Showing 42 changed files with 110 additions and 177 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"title": "Classes should not be too complex",
"title": "Cyclomatic Complexity of classes should not be too high",
"type": "CODE_SMELL",
"status": "deprecated",
"remediation": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,24 @@
<p>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 at this address or spoof the IP address. Such an attack is always
possible, but in the case of a hardcoded IP address the fix will be much slower, which will increase an attack's impact.</p>
<h2>Recommended Secure Coding Practices</h2>
<h2>Ask Yourself Whether</h2>
<p>The disclosed IP address is sensitive, eg:</p>
<ul>
<li> make the IP address configurable. </li>
<li> Can give information to an attacker about the network topology. </li>
<li> It's a personal (assigned to an identifiable person) IP address. </li>
</ul>
<h2>Noncompliant Code Example</h2>
<p>There is a risk if you answered yes to any of these questions.</p>
<h2>Recommended Secure Coding Practices</h2>
<p>Don't hard-code the IP address in the source code, instead make it configurable.</p>
<h2>Sensitive Code Example</h2>
<pre>
$socket = socket_create(AF_INET, SOCK_STREAM, SOL_TCP);
socket_connect($socket, '8.8.8.8', 23); // Sensitive
</pre>
<h2>Compliant Solution</h2>
<pre>
$socket = socket_create(AF_INET, SOCK_STREAM, SOL_TCP);
socket_connect($socket, '8.8.8.8', 23); // Noncompliant
socket_connect($socket, IP_ADDRESS, 23); // Compliant
</pre>
<h2>Exceptions</h2>
<p>No issue is reported for the following cases because they are not considered sensitive:</p>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,7 @@ <h2>Ask Yourself Whether</h2>
<li> the executed code may come from an untrusted source and hasn't been sanitized. </li>
<li> you really need to run code dynamically. </li>
</ul>
<p>You are at risk if you answered yes to the first question. You are increasing the security risks for no reason if you answered yes to the second
question.</p>
<p>There is a risk if you answered yes to any of those questions.</p>
<h2>Recommended Secure Coding Practices</h2>
<p>Regarding the execution of unknown code, the best solution is to not run code provided by an untrusted source. If you really need to do it, run the
code in a <a href="https://en.wikipedia.org/wiki/Sandbox_(computer_security)">sandboxed</a> environment. Use jails, firewalls and whatever means your
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"title": "Functions should not be too complex",
"title": "Cyclomatic Complexity of functions should not be too high",
"type": "CODE_SMELL",
"status": "ready",
"remediation": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ <h2>Ask Yourself Whether</h2>
<li> Credentials are used in production environments. </li>
<li> Application re-distribution is required before updating the credentials. </li>
</ul>
<p>You are at risk, if you answered yes to any of these questions.</p>
<p>There is a risk if you answered yes to any of those questions.</p>
<h2>Recommended Secure Coding Practices</h2>
<ul>
<li> Store the credentials in a configuration file that is not pushed to the code repository. </li>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ <h2>Ask Yourself Whether</h2>
<li> the SQL query is built using string formatting technics, such as concatenating variables. </li>
<li> some of the values are coming from an untrusted source and are not sanitized. </li>
</ul>
<p>You may be at risk if you answered yes to this question.</p>
<p>There is a risk if you answered yes to any of those questions.</p>
<h2>Recommended Secure Coding Practices</h2>
<ul>
<li> Avoid building queries manually using formatting technics. If you do it anyway, do not include user input in this building process. </li>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,14 @@ <h2>Ask Yourself Whether</h2>
<li> it's not sure that the website contains <a href="https://developer.mozilla.org/fr/docs/S%C3%A9curit%C3%A9/MixedContent">mixed content</a> or
not (ie HTTPS everywhere or not) </li>
</ul>
<p>You are at risk if you answered yes to any of those questions.</p>
<p>There is a risk if you answered yes to any of those questions.</p>
<h2>Recommended Secure Coding Practices</h2>
<ul>
<li> It is recommended to use <code>HTTPs</code> everywhere so setting the <code>secure</code> flag to <em>true</em> should be the default behaviour
when creating cookies. </li>
<li> Set the <code>secure</code> flag to <em>true</em> for session-cookies. </li>
</ul>
<h2>Sensitive Code Examples</h2>
<h2>Sensitive Code Example</h2>
<p>In <em>php.ini</em> you can specify the flags for the session cookie which is security-sensitive:</p>
<pre>
session.cookie_secure = 0; // Sensitive: this security-sensitive session cookie is created with the secure flag set to false (cookie_secure = 0)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ <h2>Ask Yourself Whether</h2>
<li> the generated value is used multiple times. </li>
<li> an attacker can access the generated value. </li>
</ul>
<p>You are at risk if you answered yes to the first question and any of the following ones.</p>
<p>There is a risk if you answered yes to any of those questions.</p>
<h2>Recommended Secure Coding Practices</h2>
<ul>
<li> Use functions which rely on a cryptographically strong random number generator such as <code>random_int()</code> or <code>random_bytes()</code>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,6 @@ <h2>See</h2>
FIO52-J.</a> - Do not store unencrypted sensitive information on the client side </li>
<li> Derived from FindSecBugs rule <a href="https://find-sec-bugs.github.io/bugs.htm#COOKIE_USAGE">COOKIE_USAGE</a> </li>
</ul>
<h2>Deprecated</h2>
<p>This rule is deprecated, and will eventually be removed.</p>

Original file line number Diff line number Diff line change
@@ -1,29 +1,16 @@
{
"title": "Writing cookies is security-sensitive",
"type": "SECURITY_HOTSPOT",
"status": "ready",
"status": "deprecated",
"remediation": {
"func": "Constant\/Issue",
"constantCost": "5min"
},
"tags": [
"cwe",
"sans-top25-porous",
"owasp-a3"

],
"defaultSeverity": "Minor",
"ruleSpecification": "RSPEC-2255",
"sqKey": "S2255",
"scope": "Main",
"securityStandards": {
"CWE": [
315,
312,
565,
807
],
"OWASP": [
"A3"
]
}
"scope": "Main"
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,6 @@ <h2>See</h2>
<li> <a href="https://www.sans.org/top25-software-errors/#cat3">SANS Top 25</a> - Porous Defenses </li>
<li> Derived from FindSecBugs rule <a href="http://h3xstream.github.io/find-sec-bugs/bugs.htm#RSA_NO_PADDING">RSA NoPadding Unsafe</a> </li>
</ul>
<h2>Deprecated</h2>
<p>This rule is deprecated; use {rule:php:S5542} instead.</p>

Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"title": "Cryptographic RSA algorithms should always incorporate OAEP (Optimal Asymmetric Encryption Padding)",
"type": "VULNERABILITY",
"status": "ready",
"status": "deprecated",
"remediation": {
"func": "Constant\/Issue",
"constantCost": "20min"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,4 +36,6 @@ <h2>See</h2>
<li> <a href="https://www.sans.org/top25-software-errors/#cat3">SANS Top 25</a> - Porous Defenses </li>
<li> Derived from FindSecBugs rule <a href="http://h3xstream.github.io/find-sec-bugs/bugs.htm#DES_USAGE">DES / DESede Unsafe</a> </li>
</ul>
<h2>Deprecated</h2>
<p>This rule is deprecated; use {rule:php:S5547} instead.</p>

Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"title": "Neither DES (Data Encryption Standard) nor DESede (3DES) should be used",
"type": "VULNERABILITY",
"status": "ready",
"status": "deprecated",
"remediation": {
"func": "Constant\/Issue",
"constantCost": "20min"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,10 @@
"constantCost": "30min"
},
"tags": [
"owasp-a3"

],
"defaultSeverity": "Major",
"ruleSpecification": "RSPEC-3011",
"sqKey": "S3011",
"scope": "Main",
"securityStandards": {
"OWASP": [
"A3"
]
}
"scope": "Main"
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,13 @@ <h2>Ask Yourself Whether</h2>
<li> the <code>HttpOnly</code> attribute offer an additional protection (not the case for an <em>XSRF-TOKEN cookie</em> / CSRF token for example)
</li>
</ul>
<p>You are at risk if you answered yes to any of those questions.</p>
<p>There is a risk if you answered yes to any of those questions.</p>
<h2>Recommended Secure Coding Practices</h2>
<ul>
<li> By default the <code>HttpOnly</code> flag should be set to <em>true</em> for most of the cookies and it's mandatory for session /
sensitive-security cookies. </li>
</ul>
<h2>Sensitive Code Examples</h2>
<h2>Sensitive Code Example</h2>
<p>In <em>php.ini</em> you can specify the flags for the session cookie which is security-sensitive:</p>
<pre>
session.cookie_httponly = 0; // Sensitive: this sensitive session cookie is created with the httponly flag set to false and so it can be stolen easily in case of XSS vulnerability
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,4 +35,6 @@ <h2>See</h2>
<li> <a href="https://www.owasp.org/index.php/Top_10-2017_A3-Sensitive_Data_Exposure">OWASP Top 10 2017 Category A3</a> - Sensitive Data Exposure
</li>
</ul>
<h2>Deprecated</h2>
<p>This rule is deprecated, and will eventually be removed.</p>

Original file line number Diff line number Diff line change
@@ -1,23 +1,16 @@
{
"title": "Creating cookies with broadly defined \"domain\" flags is security-sensitive",
"type": "SECURITY_HOTSPOT",
"status": "ready",
"status": "deprecated",
"remediation": {
"func": "Constant\/Issue",
"constantCost": "5min"
},
"tags": [
"owasp-a7",
"owasp-a3"

],
"defaultSeverity": "Info",
"ruleSpecification": "RSPEC-3331",
"sqKey": "S3331",
"scope": "Main",
"securityStandards": {
"OWASP": [
"A7",
"A3"
]
}
"scope": "Main"
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@
"securityStandards": {
"CWE": [
327,
326
326,
295
],
"OWASP": [
"A3",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@
<li> n ≥ 224 for ECDH and ECMQV (Examples: <code>secp192r1</code> is a non-compliant curve (<code>n</code> &lt; 224) but <code>secp224k1</code> is
compliant (<code>n</code> &gt;= 224)) </li>
</ul>
<p><strong>Encryption and Decryption</strong>: </p>
<p><strong>Symmetric keys</strong>: </p>
<ul>
<li> AES-128, 192, 256 </li>
<li> key length ≥ 128 bits </li>
</ul>
<p>This rule will not raise issues for ciphers that are considered weak (no matter the key size) like <code>DES</code>, <code>Blowfish</code>.</p>
<h2>Noncompliant Code Example</h2>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,28 +11,23 @@ <h2>Ask Yourself Whether</h2>
<li> the code or configuration enabling the application debug features is deployed on production servers. </li>
<li> the application runs by default with debug features activated. </li>
</ul>
<p>You are at risk if you answered yes to any of these questions.</p>
<p>There is a risk if you answered yes to any of those questions.</p>
<h2>Recommended Secure Coding Practices</h2>
<p>The application should run by default in the most secure mode, i.e. as on production servers. This is to prevent any mistake. Enabling debug mode
should be explicitly asked via a command line argument, an environment variable or a configuration file.</p>
<p>Check that every aspect of the debug mode is controlled by only one configuration switch: logging, exception/error handling, access control, etc...
It is otherwise very easy to forget one of them.</p>
<p>Do not enable debug mode on production servers.</p>
<p>Only the value "0" or "false" for CakePHP 3.x is suitable (production mode) to not leak sensitive data on the logs.</p>
<h2>Noncompliant Code Example</h2>
<p>Do not enable debug features on production servers.</p>
<h2>Sensitive Code Example</h2>
<p>CakePHP 1.x, 2.x:</p>
<pre>
Configure::write('debug', 1); // Noncompliant; development mode
Configure::write('debug', 1); // Sensitive: development mode
or
Configure::write('debug', 2); // Noncompliant; development mode
Configure::write('debug', 2); // Sensitive: development mode
or
Configure::write('debug', 3); // Noncompliant; development mode
Configure::write('debug', 3); // Sensitive: development mode
</pre>
<p>CakePHP 3.0:</p>
<pre>
use Cake\Core\Configure;

Configure::config('debug', true);
Configure::config('debug', true); // Sensitive: development mode
</pre>
<h2>Compliant Solution</h2>
<p>CakePHP 1.2:</p>
Expand All @@ -43,7 +38,7 @@ <h2>Compliant Solution</h2>
<pre>
use Cake\Core\Configure;

Configure::config('debug', false);
Configure::config('debug', false); // Compliant: "0" or "false" for CakePHP 3.x is suitable (production mode) to not leak sensitive data on the logs.
</pre>
<h2>See</h2>
<ul>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,4 +36,6 @@ <h2>See</h2>
<li> <a href="https://cwe.mitre.org/data/definitions/502.html">MITRE, CWE-502</a> - Deserialization of Untrusted Data </li>
<li> Derived from FindSecBugs rule <a href="https://find-sec-bugs.github.io/bugs.htm#OBJECT_DESERIALIZATION">OBJECT_DESERIALIZATION </a> </li>
</ul>
<h2>Deprecated</h2>
<p>This rule is deprecated, and will eventually be removed.</p>

Original file line number Diff line number Diff line change
@@ -1,25 +1,16 @@
{
"title": "Deserializing objects from an untrusted source is security-sensitive",
"type": "SECURITY_HOTSPOT",
"status": "ready",
"status": "deprecated",
"remediation": {
"func": "Constant\/Issue",
"constantCost": "15min"
},
"tags": [
"cwe",
"owasp-a8"

],
"defaultSeverity": "Critical",
"ruleSpecification": "RSPEC-4508",
"sqKey": "S4508",
"scope": "Main",
"securityStandards": {
"CWE": [
502
],
"OWASP": [
"A8"
]
}
"scope": "Main"
}
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ <h2>Ask Yourself Whether</h2>
<li> the executed regular expression is sensitive and a user can provide a string which will be analyzed by this regular expression. </li>
<li> your regular expression engine performance decrease with specially crafted inputs and regular expressions. </li>
</ul>
<p>You may be at risk if you answered yes to any of those questions.</p>
<p>There is a risk if you answered yes to any of those questions.</p>
<h2>Recommended Secure Coding Practices</h2>
<p>Do not set the constant <code>pcre.backtrack_limit</code> to a high value as it will increase the resource consumption of PCRE functions.</p>
<p>Check the error codes of PCRE functions via <code>preg_last_error</code>.</p>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -261,4 +261,6 @@ <h2>See</h2>
<li> <a href="http://cwe.mitre.org/data/definitions/327.html">MITRE, CWE-327</a> - Use of a Broken or Risky Cryptographic Algorithm </li>
<li> <a href="https://www.sans.org/top25-software-errors/#cat3">SANS Top 25</a> - Porous Defenses </li>
</ul>
<h2>Deprecated</h2>
<p>This rule is deprecated, and will eventually be removed.</p>

Original file line number Diff line number Diff line change
@@ -1,31 +1,12 @@
{
"title": "Encrypting data is security-sensitive",
"type": "SECURITY_HOTSPOT",
"status": "ready",
"status": "deprecated",
"tags": [
"cwe",
"owasp-a6",
"sans-top25-porous",
"owasp-a3"

],
"defaultSeverity": "Critical",
"ruleSpecification": "RSPEC-4787",
"sqKey": "S4787",
"scope": "Main",
"securityStandards": {
"CWE": [
321,
322,
323,
324,
325,
326,
327,
522
],
"OWASP": [
"A3",
"A6"
]
}
"scope": "Main"
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ <h2>Ask Yourself Whether</h2>
<li> Security token generation (used to confirm e-mail when registering on a website, reset password, etc ...). </li>
<li> To compute some message integrity. </li>
</ul>
<p>You are at risk if you answered yes to any of those questions.</p>
<p>There is a risk if you answered yes to any of those questions.</p>
<h2>Recommended Secure Coding Practices</h2>
<p>Safer alternatives, such as <code>SHA-256</code>, <code>SHA-512</code>, <code>SHA-3</code> or <code>bcrypt</code> are recommended, and for password
hashing, it's even better to use algorithms that not compute too "quickly", like <code>bcrypt</code> instead of <code>SHA-256</code>, because it slows
Expand Down
Loading

0 comments on commit dd01918

Please sign in to comment.