Skip to content

Commit

Permalink
Merge pull request #234 from tsmithv11/november-sast-policies
Browse files Browse the repository at this point in the history
Nov SAST policies
  • Loading branch information
anaghapamidi authored Dec 6, 2023
2 parents 9202d62 + 5bfb98d commit c42b8c6
Show file tree
Hide file tree
Showing 11 changed files with 452 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@

=== Description

User accounts with "write" permissions on an actively used repository '*' can push code directly to the default branch without requiring pull request reviews before the code is merged, potentially allowing malicious code to flow through the pipeline to production systems.
User accounts with "write" permissions on an actively used repository `*` can push code directly to the default branch without requiring pull request reviews before the code is merged, potentially allowing malicious code to flow through the pipeline to production systems.

* An actively used repository is defined as having at least two contributors, over 50 commits, and has been updated in the last 90 days.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,9 @@

=== Description

A requestor who opened a pull request can approve their own changes on an actively used repository '*'.

A requestor who opened a pull request can approve their own changes on an actively used repository `*`.


Repository policy requires approval from reviewers to merge pull requests which can result in a requestor being able to approve their own request, if the required number of reviewers is set to _1_. If the number of reviewers is set to more than 1, the requestor is considered as one of the reviewers, reducing the number of additional reviewers required to approve the pull request.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,4 +149,21 @@
|CKV3_SAST_161
|MEDIUM

|xref:sast-policy-174.adoc[Relative Path Traversal]
|CKV3_SAST_174
|HIGH

|xref:sast-policy-176.adoc[Improper Neutralization of Script-Related HTML Tags in a Web Page (Basic XSS)]
|CKV3_SAST_176
|MEDIUM

|xref:sast-policy-177.adoc[Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection')]
|CKV3_SAST_177
|MEDIUM

|xref:sast-policy-178.adoc[Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection')]
|CKV3_SAST_178
|MEDIUM


|===
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@

== Relative Path Traversal

=== Policy Details

[width=45%]
[cols="1,1"]
|===
|Prisma Cloud Policy ID
| 6f502a98-a990-4f76-95ac-43980b7f1390

|Checkov ID
|CKV3_SAST_174

|Severity
|HIGH

|Subtype
|Build

|Language
|javascript

|CWEs
|https://cwe.mitre.org/data/definitions/23.html[CWE-23: Relative Path Traversal]

|OWASP Categories
|https://owasp.org/Top10/A01_2021-Broken_Access_Control/[A01:2021 - Broken Access Control]

|===


=== Description

This policy detects when an application in JavaScript contains potential Relative Path Traversal flaw. The vulnerability occurs when the product uses external input to construct a pathname that should be within a restricted directory, but does not properly neutralize sequences such as ".." that can resolve to a location that is outside of that directory. This allows attackers to traverse the file system to access files or directories that are outside of the restricted directory.

Vulnerable code example:

[source,javascript]
----
var path = document.getElementById('path').value;
fs.writeFileSync(path, 'Hello World!');
----

This code is vulnerable because it retrieves an input from the user and uses it directly to write a file. If user’s input ended up containing special sequences like `..`, they could write a file anywhere on the server's filesystem.

=== Fix - Buildtime

To fix this vulnerability, sanitize the user’s input by removing or neutralizing the '..' path sequences using functions like 'replaceAll' or 'path.normalize' before using it in filesystem operations.

Secure code example:

[source,javascript]
----
var path = document.getElementById('path').value;
path = path.normalize(path);
fs.writeFileSync(path, 'Hello World!');
----

This code is no longer vulnerable because it uses 'path.normalize' to sanitize the user input before writing a file. This ensures that the file is written in a predetermined directory and not anywhere else on the server's filesystem.

Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@

== Improper Neutralization of Script-Related HTML Tags in a Web Page (Basic XSS)

=== Policy Details

[width=45%]
[cols="1,1"]
|===
|Prisma Cloud Policy ID
| 10578a02-321f-45ea-9101-e5c7a5b49634

|Checkov ID
|CKV3_SAST_176

|Severity
|MEDIUM

|Subtype
|Build

|Language
|javascript

|CWEs
|https://cwe.mitre.org/data/definitions/80.html[CWE-80: Improper Neutralization of Script-Related HTML Tags in a Web Page (Basic XSS)]

|OWASP Categories
|https://owasp.org/Top10/A03_2021-Injection/[A03:2021 - Injection]

|===

```
=== Description

This SAST policy detects when user inputs are not sanitized properly before being displayed in a web page, which may potentially lead to cross-site scripting (XSS) attacks. Specifically, this policy targets cases when special characters are not properly neutralized in Javascript. Some problematic code examples are using `prompt()`, `document.getElementById()`, `document.getElementsByClassName()`, and `document.querySelector()` without sanitization.

Vulnerable code example:

[source,Javascript]
----
document.getElementById('user-input').innerHTML = req.body.userInput;
----

In the above code `req.body.userInput` is a user input that is directly inserted into the web page through `innerHTML` without any sanitization. If a user inserts a malicious script as input, it will lead to an XSS attack.

=== Fix - Buildtime

Use a sanitizer. Sanitizers will neutralize special characters that could be interpreted as scripting elements. Libraries like 'sanitize-html', 'xss-filters' and 'dompurify' have methods to sanitize HTML inputs.

Secure code example:

[source,Javascript]
----
var sanitizeHtml = require('sanitize-html');
document.getElementById('user-input').innerHTML = sanitizeHtml(req.body.userInput);
----

The 'sanitize-html' method encodes special characters that have significant meanings in HTML so they cannot be interpreted as scripting elements. This way, even if the user provides a malicious script as input, it will not result in an XSS attack.
```

Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@

== Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection')

=== Policy Details

[width=45%]
[cols="1,1"]
|===
|Prisma Cloud Policy ID
| 1f8fba75-7963-405f-a86b-ef7384d58cd3

|Checkov ID
|CKV3_SAST_177

|Severity
|MEDIUM

|Subtype
|Build

|Language
|javascript

|CWEs
|https://cwe.mitre.org/data/definitions/78.html[CWE-78: Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection')]

|OWASP Categories
|https://owasp.org/Top10/A03_2021-Injection/[A03:2021 - Injection]

|===

```
=== Description

This policy detects instances where an application is using external input to construct an OS command without neutralizing special elements that could alter the intended command. This could potentially lead to OS command injection, which is a serious security vulnerability.

The policy scope is JavaScript code, and it looks for use of known insecure methods such as 'spawnSync', 'execSync', 'exec', and 'spawn'. It also takes into account if the application is interacting with Document Object Model (DOM) by using methods like 'getElementById', 'getElementsByClassName' and 'querySelector'.

Vulnerable code example:

[source,JavaScript]
----
const express = require('express');
const app = express();
const exec = require('child_process').exec;
app.get('/', function (req, res) {
let command = req.query.command;
exec(command);
});
----

The above code is vulnerable because it uses the query parameter from the request (req.query.command) to execute an OS command. This can create a potential OS command injection vulnerability if an attacker includes malicious command in the query parameter.

=== Fix - Buildtime

To fix the issue, ensure that any data used in an OS command is properly sanitized before use. Additionally, consider using safer alternatives to execute OS commands that doesn't execute shell command directly.

Secure code example:

[source,JavaScript]
----
const express = require('express');
const app = express();
const exec = require('child_process').execFile;
app.get('/', function (req, res) {
let command = req.query.command;
exec('mySafeProgram', [command]);
});
----

In the secure version of the code, even if the user input isn't fully sanitized, the application executed 'mySafeProgram' with user input as an argument, rather than executing user input directly as a command. This way, an attacker is not able to execute arbitrary commands.
```

Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@

== Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection')

=== Policy Details

[width=45%]
[cols="1,1"]
|===
|Prisma Cloud Policy ID
| fcc9a4cd-df19-4596-acc9-2ba6286dab48

|Checkov ID
|CKV3_SAST_178

|Severity
|MEDIUM

|Subtype
|Build

|Language
|javascript

|CWEs
|https://cwe.mitre.org/data/definitions/89.html[CWE-89: Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection')]

|OWASP Categories
|https://owasp.org/Top10/A03_2021-Injection/[A03:2021 - Injection]

|===

```
=== Description

This policy detects instances where an SQL command is built using externally-influenced input, but doesn't properly neutralize special elements that could modify the intended SQL command when it's sent to a downstream component. This leaves the system vulnerable to SQL Injection.

Vulnerable code example:

[source,JavaScript]
----
const mysql = require('mysql');
const connection = mysql.createConnection({
host: 'localhost',
user: 'me',
password: 'secret',
database: 'my_db'
});
connection.connect();
const userInput = prompt("Please enter your user id");
connection.query(`SELECT * FROM users WHERE id = ${userInput}`, function (error, results, fields) {
if (error) throw error;
console.log(results);
});
----
In the above code, userInput variable is coming directly from the user and being inserted into an SQL query. This can lead to SQL Injection if a user input something like "1; DROP TABLE users; --".

=== Fix - Buildtime

To fix this vulnerability, use parameterized queries.

Secure code example:

[source,JavaScript]
----
const mysql = require('mysql');
const connection = mysql.createConnection({
host: 'localhost',
user: 'me',
password: 'secret',
database: 'my_db'
});
connection.connect();
const userInput = prompt("Please enter your user id");
const sql = 'SELECT * FROM users WHERE id = ?';
connection.query(sql, [userInput], function (error, results, fields) {
if (error) throw error;
console.log(results);
});
----
Now, instead of directly embedding user input into the SQL query, we are using a parameterized query ('?' placeholder). If the user tries to input something malicious, it will simply be treated as a string, rather than part of the SQL command, protecting the system from SQL Injection.
```

Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@

This policy identifies instances in JavaScript where the `new Buffer()` constructor is used with non-literal values. Such usage can lead to potential memory leaks. The `new Buffer()` constructor is considered unsafe when used with non-literal arguments, and it's recommended to use safer alternatives such as `Buffer.from()` or `Buffer.alloc()`.

For more information about the risks and deprecation of the `new Buffer()` constructor, refer to the [Node.js documentation](https://nodejs.org/en/docs/guides/buffer-constructor-deprecation/), the [Node.js GitHub issue](https://github.com/nodejs/node/issues/4660), and the [ESLint Community Plugin](https://github.com/eslint-community/eslint-plugin-security/blob/main/rules/detect-new-buffer.js).
For more information about the risks and deprecation of the `new Buffer()` constructor, refer to the https://nodejs.org/en/docs/guides/buffer-constructor-deprecation/[Node.js documentation], the https://github.com/nodejs/node/issues/4660[Node.js GitHub issue], and the https://github.com/eslint-community/eslint-plugin-security/blob/main/rules/detect-new-buffer.js[ESLint Community Plugin].

Example of violating code:

Expand Down
Loading

0 comments on commit c42b8c6

Please sign in to comment.