Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 66 additions & 0 deletions docs/static-code-analysis-rules/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
# Ballerina Static Code Analysis Rules

This document points to the static code analysis rule files created under each Ballerina module directory. The `common` directory contains rules that affect multiple Ballerina modules.

Each rule file follows the following format:

```md
# Rule definition

- Rule Title: `Title of the rule`
- Rule ID: `fully qualified identifier of the rule/undefined`
- Rule Kind: `CODE_SMELL/BUG/VULNERABILITY`
- Affecting Modules: `['Ballerina/moduleName', 'Ballerinax/moduleName', 'Org/moduleName']`

### Why is this an issue?

Description of the issue.

### Non-compliant Code Example:

[Non-compliant code example]

### The solution can be to:

- Solution 1

[Solution 1 code example]

- Solution 2

[Solution 2 code example]

### Additional information

- Link 1 to additional information
- Link 2 to additional information
```

# Rules

- ballerina/lang:
- [Should avoid checkpanic](ballerina-lang/should-avoid-checkpanic.md)
- [Functions should not have too many parameters](ballerina-lang/functions-should-not-have-too-many-parameters.md)
- [Functions should not be empty](ballerina-lang/functions-should-not-be-empty.md)

- ballerina/file:
- [Avoid using publicly writable directories for file operations without proper access controls](ballerina-file/avoid-using-publicly-writable-directories-for-file-operations-without-proper-access-controls.md)
- [I/O function calls should not be vulnerable to path injection attacks](ballerina-file/io-function-calls-should-not-be-vulnerable-to-path-injection-attacks.md)

- ballerina/http:
- [Avoid allowing unsafe HTTP methods](ballerina-http/avoid-allowing-unsafe-HTTP-methods.md)
- [Avoid permissive Cross-Origin Resource Sharing](ballerina-http/avoid-permissive-cross-origin-resource-sharing.md)
- [Avoid creating cookies without the HttpOnly and secure flags](ballerina-http/avoid-creating-cookies-without-the-HttpOnly-and-secure-flags.md)

- ballerina/os:
- [Avoid constructing system command arguments from user input without proper sanitization](ballerina-os/avoid-constructing-system-command-arguments-from-user-input-without-proper-sanitization.md)

- ballerinax/mysql:
- [A secure password should be used when connecting to a database](ballerinax-mysql/a-secure-password-should-be-used-when-connecting-to-a-database.md)

- common:
- [Passwords should not be stored in plain text or with a fast hashing algorithm](common/passwords-should-not-be-stored-in-plain-text-or-with-a-fast-hashing-algorithm.md)

# References

- [Ballerina Static Code Analysis Rules](https://docs.google.com/document/d/16ynMq1J8Ua4OnofghVEzJH7N7JYz6axeKK4X8OFm3Lo/edit?usp=sharing)
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
# Rule definition

- Rule Title: `Avoid using publicly writable directories for file operations without proper access controls`
- Rule ID: `undefined`
- Rule Kind: `VULNERABILITY`
- Affecting Modules: `['Ballerina/file']`

### Why is this an issue?

Operating systems often have global directories with write access granted to any user. These directories serve as temporary storage locations like /tmp in Linux-based systems. However, when an application manipulates files within these directories, it becomes vulnerable to race conditions on filenames. A malicious user may attempt to create a file with a predictable name before the application does. If successful, such an attack could lead to unauthorized access, modification, corruption, or deletion of other files. This risk escalates further if the application operates with elevated permissions.

### Non-compliant Code Example:

```ballerina
string tempFolderPath = os:getEnv("TMP"); // Sensitive
check file:create(tempFolderPath + "/" + "myfile.txt"); // Sensitive
check file:getAbsolutePath(tempFolderPath + "/" + "myfile.txt"); // Sensitive

check file:createTemp("suffix", "prefix"); // Sensitive, will be in the default temporary-file directory.

check file:createTempDir((), "prefix"); // Sensitive, will be in the default temporary-file directory.
```

### The solution can be to:

- Using dedicated sub-folders

```ballerina
check file:create("./myDirectory/myfile.txt"); // Compliant
check file:getAbsolutePath("./myDirectory/myfile.txt"); // Compliant
```

### Additional information

- [OWASP - Top 10 2021 Category A1 - Broken Access Control](https://owasp.org/Top10/A01_2021-Broken_Access_Control/)
- [OWASP - Top 10 2017 Category A5 - Broken Access Control](https://owasp.org/www-project-top-ten/2017/A5_2017-Broken_Access_Control)
- [OWASP - Top 10 2017 Category A3 - Sensitive Data Exposure](https://owasp.org/www-project-top-ten/2017/A3_2017-Sensitive_Data_Exposure)
- [CWE - CWE-377 - Insecure Temporary File](https://cwe.mitre.org/data/definitions/377)
- [CWE - CWE-379 - Creation of Temporary File in Directory with Incorrect Permissions](https://cwe.mitre.org/data/definitions/379)
- [OWASP, Insecure Temporary File](https://owasp.org/www-community/vulnerabilities/Insecure_Temporary_File)
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
# Rule definition

- Rule Title: `I/O function calls should not be vulnerable to path injection attacks`
- Rule ID: `undefined`
- Rule Kind: `VULNERABILITY`
- Affecting Modules: `['Ballerina/file']`

### Why is this an issue?

Path injections occur when an application constructs a file path using untrusted data without first validating the path.

A malicious user can inject specially crafted values, like "../", to alter the intended path. This manipulation may lead the path to resolve to a location within the filesystem where the user typically wouldn't have access.

### Non-compliant Code Example:

```ballerina
listener http:Listener endpoint = new (8080);
string targetDirectory = "./path/to/target/directory/";

service / on endpoint {
resource function get deleteFile(string fileName) returns string|error {
// Noncompliant
check file:remove(targetDirectory + fileName);

// ...
}
}
```

### The solution can be to:

- Conduct validation of canonical paths.

```ballerina
listener http:Listener endpoint = new (8080);
string targetDirectory = "./path/to/target/directory/";

service / on endpoint {
resource function get deleteFile(string fileName) returns string|error {
// Compliant
// Retrieve the normalized absolute path of the user provided file
string absoluteUserFilePath = check file:getAbsolutePath(targetDirectory + fileName);
string normalizedAbsoluteUserFilePath = check file:normalizePath(absoluteUserFilePath, file:CLEAN);

// Check whether the user provided file exists
boolean fileExists = check file:test(normalizedAbsoluteUserFilePath, file:EXISTS);
if !fileExists {
return "File does not exist!";
}

// Retrieve the normalized absolute path of parent directory of the user provided file
string canonicalDestinationPath = check file:parentPath(normalizedAbsoluteUserFilePath);
string normalizedCanonicalDestinationPath = check file:normalizePath(canonicalDestinationPath, file:CLEAN);

// Retrieve the normalized absolute path of the target directory
string absoluteTargetFilePath = check file:getAbsolutePath(targetDirectory);
string normalizedTargetDirectoryPath = check file:normalizePath(absoluteTargetFilePath, file:CLEAN);

// Perform comparison of user provided file path and target directory path
boolean dirMatch = normalizedTargetDirectoryPath.equalsIgnoreCaseAscii(normalizedCanonicalDestinationPath);
if !dirMatch {
return "Entry is not in the target directory!";
}

check file:remove(normalizedAbsoluteUserFilePath);

// ...
}
}
```

### Additional information

- [OWASP - Top 10 2021 Category A1 - Broken Access Control](https://owasp.org/Top10/A01_2021-Broken_Access_Control/)
- [OWASP - Top 10 2021 Category A3 - Injection](https://owasp.org/Top10/A03_2021-Injection/)
- [OWASP - Top 10 2017 Category A1 - Injection](https://owasp.org/www-project-top-ten/2017/A1_2017-Injection)
- [OWASP - Top 10 2017 Category A5 - Broken Access Control](https://owasp.org/www-project-top-ten/2017/A5_2017-Broken_Access_Control)
- [CWE - CWE-20 - Improper Input Validation](https://cwe.mitre.org/data/definitions/20)
- [CWE - CWE-22 - Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal')](https://cwe.mitre.org/data/definitions/22)
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
# Rule definition

- Rule Title: `Avoid allowing unsafe HTTP methods`
- Rule ID: `undefined`
- Rule Kind: `VULNERABILITY`
- Affecting Modules: `['Ballerina/http']`

### Why is this an issue?

An HTTP resource is safe when used for read-only operations like GET, HEAD, or OPTIONS. An unsafe HTTP resource is used to alter the state of an application, such as modifying the user’s profile on a web application.

Unsafe HTTP resources include POST, PUT, and DELETE.

Enabling both safe and insecure HTTP resources to execute a particular operation on a web application may compromise its security; for instance, CSRF protections are typically designed to safeguard operations executed by insecure HTTP resources.

### Non-compliant Code Example:

```ballerina
listener http:Listener endpoint = new (8080);

service / on endpoint {
// Sensitive: by default all HTTP methods are allowed
resource function default deleteRequest(http:Request clientRequest, string username) returns string {
// state of the application will be changed here
// ...
}
}
```

### The solution can be to:

- For every resource in an application, it’s crucial to explicitly define the type of the HTTP resource, ensuring that safe resources are exclusively used for read-only operations.

```ballerina
service / on endpoint {
// Compliant
resource function delete deleteRequest(http:Request clientRequest, string username) returns string {
// state of the application will be changed here
// ...
}
}
```

### Additional information

- [OWASP - Top 10 2021 Category A1 - Broken Access Control](https://owasp.org/Top10/A01_2021-Broken_Access_Control/)
- [OWASP - Top 10 2021 Category A4 - Insecure Design](https://owasp.org/Top10/A04_2021-Insecure_Design/)
- [OWASP - Top 10 2017 Category A5 - Broken Access Control](https://owasp.org/www-project-top-ten/2017/A5_2017-Broken_Access_Control)
- [CWE - CWE-352 - Cross-Site Request Forgery (CSRF)](https://cwe.mitre.org/data/definitions/352)
- [OWASP: Cross-Site Request Forgery](https://owasp.org/www-community/attacks/csrf)
- [Spring Security Official Documentation: Use proper HTTP verbs (CSRF protection)](https://docs.spring.io/spring-security/site/docs/5.0.x/reference/html/csrf.html#csrf-use-proper-verbs)
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
# Rule definition

- Rule Title: `Avoid creating cookies without the 'HttpOnly' and 'secure' flags`
- Rule ID: `undefined`
- Rule Kind: `VULNERABILITY`
- Affecting Modules: `['Ballerina/http']`

### Why is this an issue?

When a cookie is configured without the HttpOnly attribute set to true or lacks the secure attribute set to true, it presents security vulnerabilities. The HttpOnly attribute is essential as it prevents client-side scripts from accessing the cookie, thereby mitigating Cross-Site Scripting (XSS) attacks aimed at stealing session cookies. Likewise, the secure attribute ensures that the cookie is not transmitted over unencrypted HTTP requests, thereby minimizing the risk of unauthorized interception during man-in-the-middle attacks.

### Non-compliant Code Example:

```ballerina
listener http:Listener endpoint = new (8080);

service / on endpoint {
resource function get example() returns http:Response|error? {
http:Response response = new http:Response();

// Sensitive: this sensitive cookie is created with the httponly flag and secure flag not defined (by default set to false) and so it can be stolen easily in case of XSS vulnerability
http:Cookie cookie = new ("COOKIENAME", "sensitivedata");
response.addCookie(cookie);

return response;
}
}
```

### The solution can be to:

- When creating session or security-sensitive cookies, it’s recommended to set both the httpOnly and secure flags to true.

```ballerina
listener http:Listener endpoint = new (8080);

service / on endpoint {
resource function get example() returns http:Response|error? {
http:Response response = new http:Response();

// Compliant: this sensitive cookie is protected against theft (HttpOnly=true), the sensitive cookie will not be sent during an unencrypted HTTP request due to the secure flag set to true
http:Cookie cookie = new (
"COOKIENAME",
"sensitivedata",
httpOnly = true, // Compliant
secure = true // Compliant
);
response.addCookie(cookie);

return response;
}
}
```

### Additional information

- [OWASP - Top 10 2021 Category A5 - Security Misconfiguration](https://owasp.org/Top10/A05_2021-Security_Misconfiguration/)
- [OWASP - Top 10 2021 Category A7 - Identification and Authentication Failures](https://owasp.org/Top10/A07_2021-Identification_and_Authentication_Failures/)
- [developer.mozilla.org - CORS](https://developer.mozilla.org/en-US/docs/Web/HTTP/CORS)
- [developer.mozilla.org - Same origin policy](https://developer.mozilla.org/en-US/docs/Web/Security/Same-origin_policy)
- [OWASP - Top 10 2017 Category A6 - Security Misconfiguration](https://owasp.org/www-project-top-ten/2017/A6_2017-Security_Misconfiguration\)
- [OWASP HTML5 Security Cheat Sheet - Cross Origin Resource Sharing](https://cheatsheetseries.owasp.org/cheatsheets/HTML5_Security_Cheat_Sheet.html#cross-origin-resource-sharing)
- [CWE - CWE-346 - Origin Validation Error](https://cwe.mitre.org/data/definitions/346)
- [CWE - CWE-942 - Overly Permissive Cross-domain Whitelist](https://cwe.mitre.org/data/definitions/942)
- [OWASP HttpOnly](https://owasp.org/www-community/HttpOnly)
- [OWASP - Top 10 2017 Category A7 - Cross-Site Scripting (XSS)](https://owasp.org/www-project-top-ten/2017/A7_2017-Cross-Site_Scripting_(XSS))
- [CWE - CWE-1004 - Sensitive Cookie Without 'HttpOnly' Flag](https://cwe.mitre.org/data/definitions/1004)
- [Derived from FindSecBugs rule HTTPONLY_COOKIE](https://find-sec-bugs.github.io/bugs.htm#HTTPONLY_COOKIE)
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
# Rule definition

- Rule Title: `Avoid permissive Cross-Origin Resource Sharing`
- Rule ID: `undefined`
- Rule Kind: `VULNERABILITY`
- Affecting Modules: `['Ballerina/http']`

### Why is this an issue?

Having a permissive Cross-Origin Resource Sharing policy is security-sensitive, and it has led in the past to the following vulnerabilities.

- [CVE-2018-0269](http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2018-0269)
- [CVE-2017-14460](http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-14460)

Browsers enforce the same-origin policy by default, as a security measure, preventing JavaScript frontends from making cross-origin HTTP requests to resources with different origins (domains, protocols, or ports). However, the target resource can include additional HTTP headers in its response, known as CORS headers, which serve as directives for the browser and modify the access control policy, effectively relaxing the same-origin policy.

### Non-compliant Code Example:

```ballerina
listener http:Listener endpoint = new (8080);

service / on endpoint {
// Noncompliant
@http:ResourceConfig {
cors: {
allowOrigins: ["*"] // Sensitive
}
}

resource function get example() returns http:Response|error? {
// Return response
}
}
```

### The solution can be to:

- The resource configuration should be configured exclusively for trusted origins and specific resources

```ballerina
listener http:Listener endpoint = new (8080);

service / on endpoint {
@http:ResourceConfig {
cors: {
allowOrigins: ["trustedwebsite.com"] // Compliant
}
}

resource function get example() returns http:Response|error? {
// Return response
}
}
```

### Additional information

- [OWASP - Top 10 2021 Category A5 - Security Misconfiguration](https://owasp.org/Top10/A05_2021-Security_Misconfiguration/)
- [OWASP - Top 10 2021 Category A7 - Identification and Authentication Failures](https://owasp.org/Top10/A07_2021-Identification_and_Authentication_Failures/)
- [developer.mozilla.org - CORS](https://developer.mozilla.org/en-US/docs/Web/HTTP/CORS)
- [developer.mozilla.org - Same origin policy](https://developer.mozilla.org/en-US/docs/Web/Security/Same-origin_policy)
- [OWASP - Top 10 2017 Category A6 - Security Misconfiguration](https://owasp.org/www-project-top-ten/2017/A6_2017-Security_Misconfiguration)
- [OWASP HTML5 Security Cheat Sheet - Cross Origin Resource Sharing](https://cheatsheetseries.owasp.org/cheatsheets/HTML5_Security_Cheat_Sheet.html#cross-origin-resource-sharing)
- [CWE - CWE-346 - Origin Validation Error](https://cwe.mitre.org/data/definitions/346)
- [CWE - CWE-942 - Overly Permissive Cross-domain Whitelist](https://cwe.mitre.org/data/definitions/942)
Loading
Loading