Skip to content

Commit

Permalink
Update rule metadata (#2033)
Browse files Browse the repository at this point in the history
  • Loading branch information
saberduck committed Jul 9, 2020
1 parent 1845c87 commit 7e7a586
Show file tree
Hide file tree
Showing 23 changed files with 72 additions and 112 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ <h2>Noncompliant Code Example</h2>
} //Noncompliant
else {
doSomethingElse();
}{code}
}
</pre>
<h2>Compliant Solution</h2>
<pre>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,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 All @@ -25,7 +24,7 @@ <h2>Recommended Secure Coding Practices</h2>
policy</a> for javascript in a web browser).</p>
<p>Do not try to create a blacklist of dangerous code. It is impossible to cover all attacks that way.</p>
<p>Avoid using dynamic code APIs whenever possible. Hard-coded code is always safer.</p>
<h2>Noncompliant Code Example</h2>
<h2>Sensitive Code Example</h2>
<pre>
let value = eval('obj.' + propName); // Sensitive
let func = Function('obj' + propName); // Sensitive
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ <h2>Noncompliant Code Example</h2>
<h2>Compliant Solution</h2>
<pre>
if (x) {
var foo = function() {}
const foo = function() {}
}
</pre>

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 @@ -26,7 +26,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 @@ -17,7 +17,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 a cryptographically strong pseudorandom number generator (CSPRNG) like <code>crypto.getRandomValues()</code>. </li>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,4 +56,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,15 +1,13 @@
{
"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",
Expand All @@ -18,16 +16,5 @@
"JAVASCRIPT",
"TYPESCRIPT"
],
"scope": "Main",
"securityStandards": {
"CWE": [
315,
312,
565,
807
],
"OWASP": [
"A3"
]
}
"scope": "Main"
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
},
"tags": [
"cwe",
"suspicious",
"redundant"
],
"defaultSeverity": "Major",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<p>Getters and setters provide a way to enforce encapsulation by providing <code>public</code> methods that give controlled access to
<code>private</code> fields. However in classes with multiple fields it is not unusual that cut and paste is used to quickly create the needed getters
and setters, which can result in the wrong field being accessed by a getter or setter.</p>
<code>private</code> fields. However in classes with multiple fields it is not unusual that copy and paste is used to quickly create the needed
getters and setters, which can result in the wrong field being accessed by a getter or setter.</p>
<p>This rule raises an issue in any of these cases:</p>
<ul>
<li> A setter does not update the field with the corresponding name. </li>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"title": "Redundant casts and not-null assertions should be avoided",
"title": "Redundant casts and non-null assertions should be avoided",
"type": "CODE_SMELL",
"status": "ready",
"remediation": {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,56 +1,32 @@
<p>OS commands are security-sensitive. For example, their use has led in the past to the following vulnerabilities:</p>
<ul>
<li> <a href="http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2018-12465">CVE-2018-12465</a> </li>
<li> <a href="http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2018-7187">CVE-2018-7187</a> </li>
</ul>
<p>Applications that execute operating system commands or execute commands that interact with the underlying system should neutralize any
externally-provided input used to construct those commands. Failure to do so could allow an attacker to execute unexpected or dangerous commands,
potentially leading to loss of confidentiality, integrity or availability.</p>
<p> </p>
<p>This rule flags code that specifies the name of the command to run. The goal is to guide security code reviews.</p>
<p>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.</p>
<h2>Ask Yourself Whether</h2>
<ul>
<li> the executed command is constructed by input that is externally-influenced, for example, user input (attacker). (*) </li>
<li> the command execution is not restricted to the right users. (*) </li>
<li> the application can be redesigned to not rely on external input to execute the command. </li>
<li> OS command name or parameters are user-controlled. </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 this questions.</p>
<p> </p>
<h2>Recommended Secure Coding Practices</h2>
<p>Restrict the control given to the user over the executed command:</p>
<ul>
<li> make the executed command part of a whitelist and reject all commands not part of this list. </li>
<li> sanitize the user input. </li>
</ul>
<p> </p>
<p>Restrict which users can have access to the command:</p>
<ul>
<li> use a firewall to protect the process running the code, and to protect the network from the command. </li>
<li> authenticate the user and allow only some users to run the command. </li>
</ul>
<p>Reduce the damage the command can do:</p>
<ul>
<li> execute the code in a sandbox environment that enforces strict boundaries between the operating system and the process. For example: a "jail".
</li>
<li> refuse to run the command if the process has too many privileges. For example: forbid running the code as "root". </li>
</ul>
<p> </p>
<p>Use functions that don't spawn a shell.</p>
<h2>Sensitive Code Example</h2>
<pre>
const cp = require('child_process');

// The following method calls are sensitive. Validate the parameter string.
cp.exec(str);
cp.execSync(str);
// A shell will be spawn in these following cases:
cp.exec(str); // Sensitive
cp.execSync(str); // Sensitive

cp.spawn(str, { shell: true }); // Sensitive
cp.spawnSync(str, { shell: true }); // Sensitive
cp.execFile(str, { shell: true }); // Sensitive
cp.execFileSync(str, { shell: true }); // Sensitive
</pre>
<h2>Compliant Solution</h2>
<pre>
const cp = require('child_process');

// The following method calls are sensitive if the shell parameter is set to true.
cp.spawn(str, { shell: true });
cp.spawnSync(str, { shell: true });
cp.execFile(str, { shell: true });
cp.execFileSync(str, { shell: true });
cp.execFile("/usr/bin/file.exe", { shell: false }); // Compliant (note that by default with execFile method, shell property is set to false)
</pre>
<h2>Exceptions</h2>
<p>No issue will be raised if the string being passed is fully hard-coded.</p>
<h2>See</h2>
<ul>
<li> <a href="https://www.owasp.org/index.php/Top_10-2017_A1-Injection">OWASP Top 10 2017 Category A1</a> - Injection </li>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"title": "Executing OS commands is security-sensitive",
"title": "Using shell interpreter when executing OS commands is security-sensitive",
"type": "SECURITY_HOTSPOT",
"status": "ready",
"remediation": {
Expand All @@ -11,7 +11,7 @@
"owasp-a1",
"sans-top25-insecure"
],
"defaultSeverity": "Critical",
"defaultSeverity": "Major",
"ruleSpecification": "RSPEC-4721",
"sqKey": "S4721",
"compatibleLanguages": [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,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>Check whether your regular expression engine (the algorithm executing your regular expression) has any known vulnerabilities. Search for
vulnerability reports mentioning the one engine you're are using.</p>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@
"status": "ready",
"tags": [
"cwe",
"owasp-a1"
"owasp-a1",
"regex"
],
"defaultSeverity": "Critical",
"ruleSpecification": "RSPEC-4784",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,4 +84,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,12 +1,9 @@
{
"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",
Expand All @@ -15,21 +12,5 @@
"JAVASCRIPT",
"TYPESCRIPT"
],
"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
@@ -1,4 +1,4 @@
<p>Permissive Cross-Origin Resource Sharing (CORS) policy is security-sensitive. It has led in the past to the following vulnerabilities:</p>
<p>Having a permissive Cross-Origin Resource Sharing policy is security-sensitive. It has led in the past to the following vulnerabilities:</p>
<ul>
<li> <a href="http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2018-0269">CVE-2018-0269</a> </li>
<li> <a href="http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-14460">CVE-2017-14460</a> </li>
Expand All @@ -15,7 +15,7 @@ <h2>Ask Yourself Whether</h2>
<li> You access control policy is dynamically defined by a user-controlled input like <a
href="https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Origin"><code>origin</code></a> header. </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> The <code>Access-Control-Allow-Origin</code> header should be set only for a trusted origin and for specific resources. </li>
Expand All @@ -30,14 +30,19 @@ <h2>Sensitive Code Example</h2>
});
srv.listen(3000);
</pre>
<p><a href="https://expressjs.com/">Express.js</a> framework with <a href="https://github.com/expressjs/cors">cors middleware</a>:</p>
<p><a href="https://www.npmjs.com/package/express">Express.js</a> framework with <a href="https://www.npmjs.com/package/cors">cors middleware</a>:</p>
<pre>
const app = express();
const cors = require('cors');

app.use((req, res, next) =&gt; {
res.header('Access-Control-Allow-Origin', '*'); // Sensitive
next();
});
let app1 = express();
app1.use(cors()); // Sensitive: by default origin is set to *

let corsOptions = {
origin: '*' // Sensitive
};

let app2 = express();
app2.use(cors(corsOptions));
</pre>
<h2>Compliant Solution</h2>
<p><a href="https://nodejs.org/api/http.html">nodejs http</a> built-in module:</p>
Expand All @@ -49,14 +54,16 @@ <h2>Compliant Solution</h2>
});
srv.listen(3000);
</pre>
<p><a href="https://expressjs.com/">Express.js</a> framework with <a href="https://github.com/expressjs/cors">cors middleware</a>:</p>
<p><a href="https://www.npmjs.com/package/express">Express.js</a> framework with <a href="https://www.npmjs.com/package/cors">cors middleware</a>:</p>
<pre>
const app = express();
const cors = require('cors');

app.use((req, res, next) =&gt; {
res.header('Access-Control-Allow-Origin', 'trustedwebsite.com'); // Compliant
next();
});
let corsOptions = {
origin: 'trustedwebsite.com' // Compliant
};

let app = express();
app.use(cors(corsOptions));
</pre>
<h2>See</h2>
<ul>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"title": "Permissive Cross-Origin Resource Sharing policy is security-sensitive",
"title": "Having a permissive Cross-Origin Resource Sharing policy is security-sensitive",
"type": "SECURITY_HOTSPOT",
"status": "ready",
"tags": [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
"S2255",
"S2259",
"S2432",
"S2583",
"S2589",
"S2681",
"S2688",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
"S930",
"S1110",
"S1116",
"S1117",
"S1119",
"S1121",
"S1125",
Expand Down Expand Up @@ -83,6 +84,7 @@
"S2424",
"S2428",
"S2432",
"S2583",
"S2589",
"S2681",
"S2685",
Expand Down Expand Up @@ -170,6 +172,7 @@
"S4624",
"S4634",
"S4721",
"S4782",
"S4784",
"S4787",
"S4790",
Expand Down

0 comments on commit 7e7a586

Please sign in to comment.