From f550f75d0f1baa54f8d43afa3c8bd689f665c3bc Mon Sep 17 00:00:00 2001 From: Tharana Wanigaratne Date: Sun, 9 Jun 2024 17:43:15 +0530 Subject: [PATCH 1/3] Add scan tool docs --- docs/static-code-analysis-tool/README.md | 181 +++++++++++ .../ScanFileConfigurations.md | 97 ++++++ .../StaticCodeAnalysisPlatformPlugin.md | 102 ++++++ .../StaticCodeAnalyzerCompilerPlugin.md | 301 ++++++++++++++++++ 4 files changed, 681 insertions(+) create mode 100644 docs/static-code-analysis-tool/README.md create mode 100644 docs/static-code-analysis-tool/ScanFileConfigurations.md create mode 100644 docs/static-code-analysis-tool/StaticCodeAnalysisPlatformPlugin.md create mode 100644 docs/static-code-analysis-tool/StaticCodeAnalyzerCompilerPlugin.md diff --git a/docs/static-code-analysis-tool/README.md b/docs/static-code-analysis-tool/README.md new file mode 100644 index 00000000..6cb25d60 --- /dev/null +++ b/docs/static-code-analysis-tool/README.md @@ -0,0 +1,181 @@ +# Ballerina Static Code Analyzer + +Welcome to the Ballerina static code analysis tool documentation. This section contains, + +1. How the Ballerina static code analysis tool works and its features. +2. How to extend the analysis functionalities of the static code analysis tool. +3. How to extend reporting analysis results to static analysis platforms. + +These documentations are intended for the developers who wish to understand the internals of the Ballerina static code analysis tool and for developers who are planning to extend features of the tool. + +# Contents + +- [Overview](#overview) +- [Features](#features) +- [Extending Static Code Analysis Tool's Behavior](#extending-static-code-analysis-tools-behavior) + - [Compiler Plugins](#compiler-plugins) + - [Platform Plugins](#platform-plugins) +- [References](#references) + +# Overview + +```mermaid +sequenceDiagram + actor User + participant Ballerina Scan Tool + participant CompilerPlugin + participant StaticCodeAnalysisPlatformPlugin + participant Platform + + User ->> Ballerina Scan Tool: Runs 'bal scan --platforms=platformName' + activate User + deactivate User + activate Ballerina Scan Tool + Ballerina Scan Tool ->> StaticCodeAnalysisPlatformPlugin: Initialize Platform Plugins + activate StaticCodeAnalysisPlatformPlugin + Ballerina Scan Tool ->> Ballerina Scan Tool: Perform core static code analysis + Ballerina Scan Tool ->> CompilerPlugin: Engage plugins that extend CompilerPlugin + activate CompilerPlugin + CompilerPlugin ->> CompilerPlugin: Perform plugin specific static code analysis + CompilerPlugin ->> Ballerina Scan Tool: Report issues + deactivate CompilerPlugin + Ballerina Scan Tool ->> StaticCodeAnalysisPlatformPlugin: Report analysis results to each platform plugin + StaticCodeAnalysisPlatformPlugin ->> Platform: Report analysis results in a platform specific format + activate Platform + deactivate Platform + deactivate StaticCodeAnalysisPlatformPlugin + deactivate Ballerina Scan Tool +``` + +The Ballerina static code analysis tool performs analysis on both single `BAL` files and Ballerina projects and identifies potential code smells, bugs, and vulnerabilities without executing them. + +The tool performs the analysis against a core set of rules based on the core language. If compiler plugins that perform additional static analysis are available, the tool will engage them and finally report the analysis results to static analysis platforms via platform plugins. + +# Features + +1. Run analysis against all Ballerina documents in a Ballerina project. + +```bash +bal scan +``` + +2. Run analysis against a standalone Ballerina file. The file path can be relative or absolute. + +```bash +bal scan main.bal +``` + +> Note: Analyzing individual Ballerina files of a package is not allowed. + +3. Run analysis and save analysis results in a specified directory. + +```bash +bal scan --target-dir="results" +``` + +4. Run analysis and generate a HTML analysis report. +```bash +bal scan --scan-report +``` + +5. View all available rules. + +```bash +bal scan --list-rules +``` + +6. Run analysis for a specific rule. + +```bash +bal scan --include-rules="ballerina:101" +``` + +7. Run analysis for a specific set of rules. + +```bash +bal scan --include-rules="ballerina:101, ballerina/io:101" +``` + +8. Exclude analysis for a specific rule. + +```bash +bal scan --exclude-rules="ballerina:101" +``` + +9. Exclude analysis for a specific set of rules. + +```bash +bal scan --exclude-rules="ballerina:101, ballerina/io:101" +``` + +10. Run analysis and report to a static analysis platform (e.g., SonarQube). + +```bash +bal scan --platforms=sonarqube +``` + +> Note: If the Platform Plugin path is not provided in a `Scan.toml` file, the tool will attempt to download the Platform Plugin for plugins developed by the Ballerina team. + +11. Run analysis and report to multiple static analysis platforms. + +```bash +bal scan --platforms="sonarqube, semgrep, codeql" +``` + +12. Configuring the tool's behavior using a configuration file. (e.g., `Scan.toml`) + +```md +📦ballerina_project + ┣ 📜.devcontainer.json + ┣ 📜.gitignore + ┣ 📜Ballerina.toml + ┣ 📜main.bal + ┗ 📜Scan.toml +``` + +For more information on setting up a configuration file, refer to [Scan File Configurations](ScanFileConfigurations.md). + +# Extending Static Code Analysis Tool's Behavior + +The Ballerina Static Code Analysis tool provides the following methods to extend its behavior, + +- Compiler Plugins - plugins that extend the `CompilerPlugin` class can introduce custom rules. +- Platform Plugins - plugins that extend the `StaticCodeAnalysisPlatformPlugin` interface can report analysis results to static analysis platforms. + +> Note: Library modules will also implement compiler plugins to introduce library-specific rules. This enables library developers to independently develop and maintain rules applicable to their modules. + +## Compiler Plugins + +Analysis and reporting of additional issues can be done by implementing plugins that extend the Ballerina `CompilerPlugin` class. + +This will be used by, + +- Ballerina platform library developers (e.g., ballerina/http, ballerinax/twilio, etc.). +- External users who want to introduce and share custom rules. + +As compiler plugins are Ballerina packages, users and organizations can make them available via Ballerina Central. + +For more information on performing additional analysis and reporting issues, refer to creating a [Static Code Analyzer Compiler Plugin.](StaticCodeAnalyzerCompilerPlugin.md) + +## Platform Plugins + +For reporting analysis results to static code analysis platforms, the `StaticCodeAnalysisPlatformPlugin` interface can be implemented. + +```java +package io.ballerina.scan; + +public interface StaticCodeAnalysisPlatformPlugin { + String platform(); + void init(PlatformPluginContext platformPluginContext); + void onScan(List issues); +} +``` + +The scan tool will initialize each platform plugin by calling their `init` method. It uses the `platform` method to identify the platform plugin to pass platform-specific arguments, provided by the user (specified via CLI or '`Scan.toml`'), via the platform plugin context during initialization. After completing the scans, the tool will invoke the Platform Plugins' `onScan` method to notify identified issues. The plugin can then report issues to the relevant platform in the expected format. + +For more information on reporting analysis results to static analysis platforms, refer to creating a [Static Code Analysis Platform Plugin.](StaticCodeAnalysisPlatformPlugin.md) + +# References + +- [Static Code Analysis Support for Ballerina](https://docs.google.com/document/d/1J9Un9zJ05ISLnO1x1olhBqEjmMXqzcfzHnUPoyWwKXQ/edit?usp=sharing) +- [Ballerina Scan Tool](https://docs.google.com/document/d/1l_XvRGPaTJ7YOnn-HQ07erjJQDKjilFRE_mtyga3C-I/edit?usp=sharing) \ No newline at end of file diff --git a/docs/static-code-analysis-tool/ScanFileConfigurations.md b/docs/static-code-analysis-tool/ScanFileConfigurations.md new file mode 100644 index 00000000..e53ae49b --- /dev/null +++ b/docs/static-code-analysis-tool/ScanFileConfigurations.md @@ -0,0 +1,97 @@ +# Scan File Configurations + +# Contents + +- [Overview](#overview) +- [Configuration for Platform Plugins](#configuration-for-platform-plugins) +- [Configuration for Static Code Analyzer Plugins](#configuration-for-static-code-analyzer-plugins) +- [Configuration for Static Code Analyzer Rules](#configuration-for-static-code-analyzer-rules) +- [References](#references) + +# Overview + +A `Scan.toml` file can be created to configure the behavior of the Ballerina static code analysis tool. The configuration file is picked if there’s one in the working directory. + +```text +📦ballerina_project + ┣ 📜.devcontainer.json + ┣ 📜.gitignore + ┣ 📜Ballerina.toml + ┣ 📜main.bal + ┗ 📜Scan.toml +``` + +The path to the configuration file can also be specified in the `Ballerina.toml` file as follows: + +```toml +[scan] +configPath = "path/to/Scan.toml" +``` + +# Configuration for Platform Plugins + +Users can configure platform plugins in a `Scan.toml` file by specifying the plugin JAR path to report analysis results. + +```toml +[[platform]] +name = "sonarqube" +path = "path/to/sonar_platform_plugin" +``` + +If the `path` is not specified, the scan tool will attempt to download the Platform Plugins developed by the Ballerina team. + +```toml +[[platform]] +name = "sonarqube" +``` + +Users can also provide additional arguments to the platform plugin by specifying them in the `Scan.toml` file. + +```toml +[[platform]] +name = "sonarqube" +path = "path/to/sonar_platform_plugin" +sonarProjectPropertiesPath = "sonar-project.properties" +``` + +# Configuration for Static Code Analyzer Plugins + +Users can configure static code analyzer plugins in a `Scan.toml` file. These plugins are automatically added as imports by a code generator during scans, ensuring they are only engaged during a `bal scan`. + +```toml +[[analyzer]] +org = "org1" +name = "name1" +``` + +For locally available static code analyzer plugins, the following configuration can be used: + +```toml +[[analyzer]] +org = "org2" +name = "name2" +version = "version" +repository = "local" +``` + +# Configuration for Static Code Analyzer Rules + +Users can specify the rules to filter out specific issues during an analysis by defining them in a `Scan.toml` file. + +Define the rules to include in the analysis: + +```toml +[rules] +include = ["ballerina:101", "ballerina/io:101"] +``` + +Define the rules to exclude in the analysis: + +```toml +[rules] +exclude = ["ballerina:101", "ballerina/io:101"] +``` + +# References + +- [Ballerina Scan Tool](https://docs.google.com/document/d/1l_XvRGPaTJ7YOnn-HQ07erjJQDKjilFRE_mtyga3C-I/edit?usp=sharing) \ No newline at end of file diff --git a/docs/static-code-analysis-tool/StaticCodeAnalysisPlatformPlugin.md b/docs/static-code-analysis-tool/StaticCodeAnalysisPlatformPlugin.md new file mode 100644 index 00000000..57849ad7 --- /dev/null +++ b/docs/static-code-analysis-tool/StaticCodeAnalysisPlatformPlugin.md @@ -0,0 +1,102 @@ +# Static Code Analysis Platform Plugin + +# Contents + +- [What is a Static Code Analysis Platform Plugin?](#what-is-a-static-code-analysis-platform-plugin) +- [Steps to create and report analysis results to a platform from a Static Code Analysis Platform Plugin](#steps-to-create-and-report-analysis-results-to-a-platform-from-a-static-code-analysis-platform-plugin) +- [References](#references) + +# What is a Static Code Analysis Platform Plugin? + +```mermaid +sequenceDiagram + participant Ballerina Scan Tool + participant Static Code Analysis Platform Plugin + participant Platform + + Ballerina Scan Tool ->> Static Code Analysis Platform Plugin: Initialize platform plugin + activate Ballerina Scan Tool + activate Static Code Analysis Platform Plugin + Ballerina Scan Tool ->> Ballerina Scan Tool: Perform core and external static code analysis + Ballerina Scan Tool ->> Static Code Analysis Platform Plugin: Send analysis results + Static Code Analysis Platform Plugin ->> Platform: Report analysis results in a platform-specific format + activate Platform + deactivate Platform + deactivate Static Code Analysis Platform Plugin + deactivate Ballerina Scan Tool +``` + +A Static Code Analysis Platform Plugin is an implementation of the `StaticCodeAnalysisPlatformPlugin` that allows the static code analysis tool to pass analysis results to a specific static code analysis platform. + +To report analysis issues to a specific platform, the user has to specify the details of the platform plugin in a `Scan.toml` file. + +```toml +[[platforms]] +name = "platformName" +path = "path/to/platform/plugin" +``` + +Once the user defines `StaticCodeAnalysisPlatformPlugin`'s in the `Scan.toml` file, and executes `bal scan`, the static code analysis tool will first initialize the platform plugins. If additional properties are defined under each platform in the configuration file, they will be passed to the platform plugin during this stage. Then, after performing the core and external static code analysis, the tool will send the analysis results to the platform plugins. The platform plugins will then report the analysis results in a platform-specific format to the platforms. + +# Steps to create and report analysis results to a platform from a Static Code Analysis Platform Plugin + +1. Implement the `StaticCodeAnalysisPlatformPlugin` interface and build the platform plugin. + +```java +package io.ballerina.scan.platform; + +public class PlatformPlugin implements ScannerPlatformPlugin { + @Override + public String platform() { + return "platformName"; + } + + @Override + public void init(PlatformPluginContext platformPluginContext) { + } + + @Override + public void onScan(List issues) { + } +} +``` + +The `ScannerPlatformPlugin` interface has provides the following methods: + +- `platform()` - Returns the name of the platform. +- `init(PlatformPluginContext platformPluginContext)` - Initializes the platform plugin with configurations from the `PlatformPluginContext`. +- `onScan(List issues)` - Pass analysis results to the platform plugin. + +The `PlatformPluginContext` consists of platform-specific configurations that are passed to the platform plugin during initialization. + +```java +public interface PlatformPluginContext{ + Map platformArgs(); + boolean initiatedByPlatform(); +} +``` + +It has the following methods: + +- `platformArgs()` - Returns in-memory representation of the platform-specific arguments defined in the `Scan.toml` file. +- `initiatedByPlatform()` - Returns true if the analysis is triggered by the platform side and false if it is triggered by the static code analysis tool. + +2. Define the path to the platform plugin in the `Scan.toml` file. + +```toml +[[platforms]] +name = "platformName" +path = "path/to/platform/plugin" +``` + +> Note: The name provided in the `Scan.toml` file should match the platform name returned by the `platform()` method in the platform plugin. + +3. Trigger an analysis using the static code analysis tool. + +```bash +bal scan +``` + +# References + +- [SonarQube Support for Ballerina](https://docs.google.com/document/d/1AJYNN5fv9MU0UT9WKbFKc44THnhiBJhQY_cHRn6iXto/edit?usp=sharing) \ No newline at end of file diff --git a/docs/static-code-analysis-tool/StaticCodeAnalyzerCompilerPlugin.md b/docs/static-code-analysis-tool/StaticCodeAnalyzerCompilerPlugin.md new file mode 100644 index 00000000..8db76552 --- /dev/null +++ b/docs/static-code-analysis-tool/StaticCodeAnalyzerCompilerPlugin.md @@ -0,0 +1,301 @@ +# Static Code Analyzer Compiler Plugin + +# Contents + +- [What is a static code analyzer compiler plugin?](#what-is-a-static-code-analyzer-compiler-plugin) +- [Components of the Static Code Analyzer Compiler Plugin](#components-of-the-static-code-analyzer-compiler-plugin) + - [`Rule` interface](#rule-interface) + - [`RuleFactory` class](#rulefactory-class) + - [`ScannerContext` interface](#scannercontext-interface) + - [`Reporter` interface](#reporter-interface) +- [Steps to create a Static Code Analyzer Compiler Plugin](#steps-to-create-a-static-code-analyzer-compiler-plugin) + - [Step 1: Defining the static code analysis rules](#step-1-defining-the-static-code-analysis-rules) + - [Step 2: Implementing the compiler plugin classes to perform static code analysis](#step-2-implementing-the-compiler-plugin-classes-to-perform-static-code-analysis) +- [References](#references) + +# What is a static code analyzer compiler plugin? + +```mermaid +sequenceDiagram + participant Ballerina Scan Tool + participant CustomStaticCodeAnalyzerPlugin + participant CompilerPluginCache + + Ballerina Scan Tool ->> Ballerina Scan Tool: Perform core static code analysis + activate Ballerina Scan Tool + Ballerina Scan Tool ->> CustomStaticCodeAnalyzerPlugin: + activate CustomStaticCodeAnalyzerPlugin + note over Ballerina Scan Tool, CustomStaticCodeAnalyzerPlugin: Parse 'rules.json' file and create in-memory rules + CustomStaticCodeAnalyzerPlugin ->> Ballerina Scan Tool: + deactivate CustomStaticCodeAnalyzerPlugin + Ballerina Scan Tool ->> CompilerPluginCache: Create and pass a ScannerContext + activate CompilerPluginCache + Ballerina Scan Tool ->> CustomStaticCodeAnalyzerPlugin: Engage through package compilation + activate CustomStaticCodeAnalyzerPlugin + CustomStaticCodeAnalyzerPlugin ->> CompilerPluginCache: + note over CustomStaticCodeAnalyzerPlugin, CompilerPluginCache: Get ScannerContext + CompilerPluginCache ->> CustomStaticCodeAnalyzerPlugin: + CustomStaticCodeAnalyzerPlugin ->> CustomStaticCodeAnalyzerPlugin: Perform scans and reports issues via the Reporter from the ScannerContext + deactivate CustomStaticCodeAnalyzerPlugin + Ballerina Scan Tool ->> CompilerPluginCache: + note over Ballerina Scan Tool, CompilerPluginCache: Retrieve issues + CompilerPluginCache ->> Ballerina Scan Tool: + deactivate CompilerPluginCache + deactivate Ballerina Scan Tool +``` + +A static code analyzer compiler plugin is a Ballerina `CompilerPlugin` that allows extending the static code analysis features of the Ballerina scan tool. + +To perform additional analysis, the user has to specify the details of the compiler plugin in a `Scan.toml` file. + +```toml +[[analyzer]] +org = "arc" +name = "custom_static_code_analyzer" +``` + +Once the user defines `CompilerPlugin`'s in the `Scan.toml` file, and executes `bal scan`, core scans will be performed first. Then the tool generates the `CompilerPlugin`'s defined in the `Scan.toml` file as imports to an in-memory Ballerina file in the default module. Next the tool retrieves and parses the `rules.json` file of the `CompilerPlugin`'s and creates a `ScannerContext`. The tool passes the created `ScannerContext`'s to the `CompilerPluginCache` and engages the compiler plugins through package compilation. The engaged `CompilerPlugin`'s perform additional static analysis and reports issues via the `Reporter`'s retrieved from the `ScannerContext`'s. Finally, the tool retrieves the reported issues and saves them to a report file. + +# Components of the Static Code Analyzer Compiler Plugin + +The static code analysis tool provides the following components to support additional static code analysis via compiler plugins: + +- `Rule` interface. +- `RuleFactory` class. +- `ScannerContext` interface. +- `Reporter` interface. + +## `Rule` interface + +```java +package io.ballerina.scan; + +public interface Rule { + String id(); + int numericId(); + String description(); + RuleKind kind(); +} +``` + +The `Rule` interface consists of the following methods: + +- `id()` - Returns the fully qualified identifier of the rule. +- `numericId()` - Returns the numeric identifier of the rule. +- `description()` - Returns the description of the rule. +- `kind()` - Returns the `RuleKind` of the rule. + +The `RuleKind` is an enum that represents the type of the rule. + +```java +package io.ballerina.scan; + +public enum RuleKind { + CODE_SMELL, + BUG, + VULNERABILITY +} +``` + +It consists of the following types: + +- `CODE_SMELL` - Rules related to code maintainability. +- `BUG` - Rules related to coding mistakes that cause errors or unexpected behaviour at runtime. +- `VULNERABILITY` - Rules related to code susceptible to exploits due to security weaknesses. + +## `RuleFactory` class + +```java +package io.ballerina.scan; + +class RuleFactory { + private RuleFactory() {} + + static Rule createRule(int numericId, String description, RuleKind ruleKind, String org, String name) { + // Create rule with fully qualified identifier + // ... + } +} +``` + +When the `createRule` method of the `RulesFactory` is called by the static code analysis tool, it combines the organization and package names of the compiler plugin with the rules’ numeric identifier to create a `Rule` with a unique fully qualified identifier. + +## `ScannerContext` interface + +```java +package io.ballerina.scan; + +public interface ScannerContext { + Reporter getReporter(); +} +``` + +When the `getReporter` method of the `ScannerContext` is called by the static code analyzer compiler plugin, it returns the `Reporter` interface that allows reporting static analysis issues. + +The static code analysis tool creates a new scanner context that contains the in-memory rules for each static code analyzer plugin for reporting issues. + +## `Reporter` interface + +```java +package io.ballerina.scan; + +public interface Reporter { + void reportIssue(Document reportedDocument, Location location, int ruleId); + + void reportIssue(Document reportedDocument, Location location, Rule rule); +} +``` + +It consists of the following methods: + +- `reportIssue(Document, Location, int)` - allows reporting static analysis issues by accepting the numeric identifier of a rule. +- `reportIssue(Document, Location, Rule)` - allows reporting static analysis issues by accepting a programmatically created rule. + +The reporters’ `reportIssue` method accepts the numeric identifier of a rule, matches it to the fully qualified identifier of the corresponding in-memory rule, and reports the `Issue` associated with that rule. + +An `Issue` is the in-memory representation of a static code analysis issue reported by the `Reporter`. + +```java + +package io.ballerina.scan; + +public interface Issue { + Location location(); + Rule rule(); + Source source(); +} +``` + +It consists of the following methods: + +- `location()` - Returns the [Location](https://github.com/ballerina-platform/ballerina-lang/blob/fcab497029f10a9424fa7c26f9592248d57843ee/compiler/ballerina-tools-api/src/main/java/io/ballerina/tools/diagnostics/Location.java) of the issue. +- `rule()` - Returns the `Rule` associated with the issue. +- `source()` - Returns the `Source` of the issue. + +The `Source` interface represents the source of the issue. + +```java +package io.ballerina.scan; + +public enum Source { + BUILT_IN, + EXTERNAL +} +``` + +It consists of the following types: + +- `BUILT_IN` - Label for marking issues reported by the static code analysis tool and Ballerina platform static code analyzer plugins. +- `EXTERNAL` - Label for marking issues reported by non-Ballerina platform static code analyzer plugins. + +# Steps to create a Static Code Analyzer Compiler Plugin + +There are two steps to follow to create a static code analyzer compiler plugin: + +- Defining the static code analysis rules. +- Implementing the compiler plugin classes to perform static code analysis. + +## Step 1: Defining the static code analysis rules + +Create a `rules.json` file in the resources directory with static analysis rules defined in the following format: + +```json +[ + { + "id": 1, + "ruleKind": "CODE_SMELL", + "description": "rule 1" + }, + { + "id": 2, + "ruleKind": "BUG", + "description": "rule 2" + }, + { + "id": 3, + "ruleKind": "VULNERABILITY", + "description": "rule 3" + } +] +``` + +Each static code analysis rule consists of three parts: + +- `id` - Integer numeric identifier of the rule. +- `ruleKind` - Can be one of the following types: + - CODE_SMELL + - BUG + - VULNERABILITY +- `description` - Description of the rule. + +## Step 2: Implementing the compiler plugin classes to perform static code analysis + +- Create a plugin class that extends the `CompilerPlugin` class, and passes the `ScannerContext` retrieved from the `CompilerPluginContext`: + +```java +package org.arc.scanner; + +public class CustomStaticCodeAnalyzer extends CompilerPlugin { + @Override + public void init(CompilerPluginContext compilerPluginContext) { + Object context = compilerPluginContext.userData().get("ScannerContext"); + + // Passing the ScannerContext through a conditional block to avoid class loading exceptions + if (context != null) { + ScannerContext scannerContext = (ScannerContext) context; + compilerPluginContext.addCodeAnalyzer(new CustomCodeAnalyzer(scannerContext)); + } + } +} +``` + +> Note: compiler plugins of Ballerina modules should wrap the logic to retrieve and pass the `ScannerContext` in a conditional block to avoid potential class loading exceptions when running them with distribution packed commands like `bal run`. + +- Create an analyzer class that extends the `CodeAnalyzer` class, and passes the `ScannerContext` to an analysis task: + +```java +package org.arc.scanner; + +public class CustomCodeAnalyzer extends CodeAnalyzer { + private final ScannerContext scannerContext; + + public CustomCodeAnalyzer(ScannerContext scannerContext) { + this.scannerContext = scannerContext; + } + + @Override + public void init(CodeAnalysisContext codeAnalysisContext) { + codeAnalysisContext.addSyntaxNodeAnalysisTask(new CustomAnalysisTask(scannerContext), SyntaxKind.FUNCTION_BODY_BLOCK); + } +} +``` + +- Create an analysis task class that extends the `SyntaxNodeAnalysisTask` class, and performs static code analysis: + +```java +package org.arc.scanner; + +public class CustomAnalysisTask implements AnalysisTask { + private final Reporter reporter; + + public CustomAnalysisTask(ScannerContext scannerContext) { + this.reporter = scannerContext.getReporter(); + } + + @Override + public void perform(SyntaxNodeAnalysisContext context) { + Module module = context.currentPackage().module(context.moduleId()); + Document document = module.document(context.documentId()); + FunctionBodyBlockNode functionBodyBlockNode = (FunctionBodyBlockNode) context.node(); + + // Perform analysis + // ... + + reporter.reportIssue(document, functionBodyBlockNode.location(), 1); + } +} +``` + +# References + +- [Static Code Analyzer Compiler Plugin](https://docs.google.com/document/d/1yybxymfb2wWQEf2eOgTmIg6ebjY8mFSpElu37hL5lAQ/edit?usp=sharing) \ No newline at end of file From c64aa2a73f4c09883f33d7978f3b59d08c693ce2 Mon Sep 17 00:00:00 2001 From: Tharana Wanigaratne Date: Sun, 9 Jun 2024 19:39:04 +0530 Subject: [PATCH 2/3] Add static code analysis rule docs --- docs/static-code-analysis-rules/README.md | 66 ++++++++++ ...erations-without-proper-access-controls.md | 40 ++++++ ...be-vulnerable-to-path-injection-attacks.md | 79 ++++++++++++ .../avoid-allowing-unsafe-HTTP-methods.md | 51 ++++++++ ...s-without-the-HttpOnly-and-secure-flags.md | 68 ++++++++++ ...ermissive-cross-origin-resource-sharing.md | 65 ++++++++++ .../functions-should-not-be-empty.md | 26 ++++ ...ons-should-not-have-too-many-parameters.md | 121 ++++++++++++++++++ .../ballerina-lang/should-avoid-checkpanic.md | 60 +++++++++ ...-user-input-without-proper-sanitization.md | 52 ++++++++ ...d-be-used-when-connecting-to-a-database.md | 61 +++++++++ ...n-text-or-with-a-fast-hashing-algorithm.md | 108 ++++++++++++++++ 12 files changed, 797 insertions(+) create mode 100644 docs/static-code-analysis-rules/README.md create mode 100644 docs/static-code-analysis-rules/ballerina-file/avoid-using-publicly-writable-directories-for-file-operations-without-proper-access-controls.md create mode 100644 docs/static-code-analysis-rules/ballerina-file/io-function-calls-should-not-be-vulnerable-to-path-injection-attacks.md create mode 100644 docs/static-code-analysis-rules/ballerina-http/avoid-allowing-unsafe-HTTP-methods.md create mode 100644 docs/static-code-analysis-rules/ballerina-http/avoid-creating-cookies-without-the-HttpOnly-and-secure-flags.md create mode 100644 docs/static-code-analysis-rules/ballerina-http/avoid-permissive-cross-origin-resource-sharing.md create mode 100644 docs/static-code-analysis-rules/ballerina-lang/functions-should-not-be-empty.md create mode 100644 docs/static-code-analysis-rules/ballerina-lang/functions-should-not-have-too-many-parameters.md create mode 100644 docs/static-code-analysis-rules/ballerina-lang/should-avoid-checkpanic.md create mode 100644 docs/static-code-analysis-rules/ballerina-os/avoid-constructing-system-command-arguments-from-user-input-without-proper-sanitization.md create mode 100644 docs/static-code-analysis-rules/ballerinax-mysql/a-secure-password-should-be-used-when-connecting-to-a-database.md create mode 100644 docs/static-code-analysis-rules/common/passwords-should-not-be-stored-in-plain-text-or-with-a-fast-hashing-algorithm.md diff --git a/docs/static-code-analysis-rules/README.md b/docs/static-code-analysis-rules/README.md new file mode 100644 index 00000000..b88bfd6e --- /dev/null +++ b/docs/static-code-analysis-rules/README.md @@ -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) \ No newline at end of file diff --git a/docs/static-code-analysis-rules/ballerina-file/avoid-using-publicly-writable-directories-for-file-operations-without-proper-access-controls.md b/docs/static-code-analysis-rules/ballerina-file/avoid-using-publicly-writable-directories-for-file-operations-without-proper-access-controls.md new file mode 100644 index 00000000..0fc1d35c --- /dev/null +++ b/docs/static-code-analysis-rules/ballerina-file/avoid-using-publicly-writable-directories-for-file-operations-without-proper-access-controls.md @@ -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) \ No newline at end of file diff --git a/docs/static-code-analysis-rules/ballerina-file/io-function-calls-should-not-be-vulnerable-to-path-injection-attacks.md b/docs/static-code-analysis-rules/ballerina-file/io-function-calls-should-not-be-vulnerable-to-path-injection-attacks.md new file mode 100644 index 00000000..5e7b7602 --- /dev/null +++ b/docs/static-code-analysis-rules/ballerina-file/io-function-calls-should-not-be-vulnerable-to-path-injection-attacks.md @@ -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) \ No newline at end of file diff --git a/docs/static-code-analysis-rules/ballerina-http/avoid-allowing-unsafe-HTTP-methods.md b/docs/static-code-analysis-rules/ballerina-http/avoid-allowing-unsafe-HTTP-methods.md new file mode 100644 index 00000000..067a761c --- /dev/null +++ b/docs/static-code-analysis-rules/ballerina-http/avoid-allowing-unsafe-HTTP-methods.md @@ -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) \ No newline at end of file diff --git a/docs/static-code-analysis-rules/ballerina-http/avoid-creating-cookies-without-the-HttpOnly-and-secure-flags.md b/docs/static-code-analysis-rules/ballerina-http/avoid-creating-cookies-without-the-HttpOnly-and-secure-flags.md new file mode 100644 index 00000000..ff4b314d --- /dev/null +++ b/docs/static-code-analysis-rules/ballerina-http/avoid-creating-cookies-without-the-HttpOnly-and-secure-flags.md @@ -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) \ No newline at end of file diff --git a/docs/static-code-analysis-rules/ballerina-http/avoid-permissive-cross-origin-resource-sharing.md b/docs/static-code-analysis-rules/ballerina-http/avoid-permissive-cross-origin-resource-sharing.md new file mode 100644 index 00000000..7cf8b44c --- /dev/null +++ b/docs/static-code-analysis-rules/ballerina-http/avoid-permissive-cross-origin-resource-sharing.md @@ -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) \ No newline at end of file diff --git a/docs/static-code-analysis-rules/ballerina-lang/functions-should-not-be-empty.md b/docs/static-code-analysis-rules/ballerina-lang/functions-should-not-be-empty.md new file mode 100644 index 00000000..674914c3 --- /dev/null +++ b/docs/static-code-analysis-rules/ballerina-lang/functions-should-not-be-empty.md @@ -0,0 +1,26 @@ +# Rule definition + +- Rule Title: `Functions should not be empty` +- Rule ID: `undefined` +- Rule Kind: `CODE_SMELL` +- Affecting Modules: `['Ballerina/lang']` + +### Why is this an issue? + +There are multiple factors contributing to a function lacking a function body: + +- It’s an unintentional omission and should be fixed to prevent unexpected behavior in production +- It either hasn't been implemented yet or won't be supported at all. In such instances, a panic should be raised. + +### Non-compliant Code Example: + +```ballerina +// Noncompliant +public function emptyFunction(){ + +} +``` + +### The solution can be to: + +### Additional information \ No newline at end of file diff --git a/docs/static-code-analysis-rules/ballerina-lang/functions-should-not-have-too-many-parameters.md b/docs/static-code-analysis-rules/ballerina-lang/functions-should-not-have-too-many-parameters.md new file mode 100644 index 00000000..f84b3c8e --- /dev/null +++ b/docs/static-code-analysis-rules/ballerina-lang/functions-should-not-have-too-many-parameters.md @@ -0,0 +1,121 @@ +# Rule definition + +- Rule Title: `Functions should not have too many parameters` +- Rule ID: `undefined` +- Rule Kind: `CODE_SMELL` +- Affecting Modules: `['Ballerina/lang']` + +### Why is this an issue? + +Functions with lengthy parameter lists can be challenging to utilize, potentially compromising code readability and increasing the likelihood of errors. + +### Non-compliant Code Example: + +```ballerina +// Noncompliant +public function setCoordinates(int x1, int y1, int x2, int y2, int x3, int y3, int width, int height, int depth) returns int { + // ... +} + +public function main() { + int result = setCoordinates(1, 1, 2, 2, 1, 2, 3, 3, 3); + // ... +} +``` + +### The solution can be to: + +- Split the function into smaller ones. + +```ballerina +public function setOrigin1(int x1, int y1) returns int { + // ... +} + +public function setOrigin2(int x2, int y2) returns int { + // ... +} + +public function setOrigin3(int x3, int y3) returns int { + // ... +} + +public function setSize(int shapeID, int width, int height, int depth) returns int { + // ... +} +``` + +- Use record-typed parameters that group data in a way that makes sense for the specific application domain. + +```ballerina +public type Point record {| + int x; + int y; +|}; + +public type ShapeProperties record {| + int width; + int height; + int depth; +|}; + +public function setCoordinates(Point p1, Point p2, Point p3) returns int { + // ... +} + +public function setSize(int shapeID, ShapeProperties properties) returns int { + // ... +} + +public function main() { + Point p1 = {x: 1, y: 1}; + Point p2 = {x: 2, y: 2}; + Point p3 = {x: 1, y: 2}; + + int shapeID = setCoordinates(p1, p2, p3); + + ShapeProperties properties = {width: 3, height: 3, depth: 3}; + int result = setSize(shapeID, properties); + // ... +} +``` + +- Use included record parameters that allow grouping and passing by name. + +```ballerina +public type Point record {| + int x; + int y; +|}; + +public type ShapeProperties record {| + int width; + int height; + int depth; +|}; + +public type Coordinates record {| + Point p1; + Point p2; + Point p3; +|}; + +public function setCoordinates2(*Coordinates coordinates) returns int { + // ... +} + +public function setSize2(int shapeID, *ShapeProperties properties) returns int { + // ... +} + +public function compliantSolution2() { + int shapeID = setCoordinates(p1 = {x: 1, y: 2}, + p2 = {x: 1, y: 2}, + p3 = {x: 1, y: 2}); + + int result = setSize2(shapeID, {width: 3, height: 3, depth: 3}); + // ... +} +``` + +### Additional information \ No newline at end of file diff --git a/docs/static-code-analysis-rules/ballerina-lang/should-avoid-checkpanic.md b/docs/static-code-analysis-rules/ballerina-lang/should-avoid-checkpanic.md new file mode 100644 index 00000000..4c69bb11 --- /dev/null +++ b/docs/static-code-analysis-rules/ballerina-lang/should-avoid-checkpanic.md @@ -0,0 +1,60 @@ +# Rule definition + +- Rule Title: `Should avoid checkpanic` +- Rule ID: `ballerina:1` +- Rule Kind: `CODE_SMELL` +- Affecting Modules: `['Ballerina/lang']` + +### Why is this an issue? + +When “checkpanic” is used, the program terminates abruptly with a panic unless it’s handled explicitly along the call stack. + +### Non-compliant Code Example: + +```ballerina +// Noncompliant +public function checkResult() { + json result = checkpanic getResult(); +} + +public function getResult() returns json|error { + // ... +} +``` + +### The solution can be to: + +- Check and handle the error explicitly + +```ballerina +// Compliant +public function checkResult() { + json|error result = getResult(1, 2); + + if result is error { + // handle error + } +} + +public function getResult() returns json|error { + // ... +} +``` + +- Make use of the check keyword, which returns the error or transfers control to an on-fail block, in contrast to checkpanic and panicking if an expression or action evaluates to an error. + +```ballerina +// Compliant +public function checkResult() returns error? { + json result = check getResult(1, 2); +} + +public function getResult() returns json|error { + // ... +} +``` + +### Additional information + +- [Ballerina Checking expression](https://ballerina.io/spec/lang/2021R1/#section_6.33) +- [Usage of checkpanic](https://learn-ballerina.github.io/best_practices/avoid_unnecessary_panic.html#usage-of-checkpanic) \ No newline at end of file diff --git a/docs/static-code-analysis-rules/ballerina-os/avoid-constructing-system-command-arguments-from-user-input-without-proper-sanitization.md b/docs/static-code-analysis-rules/ballerina-os/avoid-constructing-system-command-arguments-from-user-input-without-proper-sanitization.md new file mode 100644 index 00000000..e55d8d50 --- /dev/null +++ b/docs/static-code-analysis-rules/ballerina-os/avoid-constructing-system-command-arguments-from-user-input-without-proper-sanitization.md @@ -0,0 +1,52 @@ +# Rule definition + +- Rule Title: `Avoid constructing system command arguments from user input without proper sanitization` +- Rule ID: `undefined` +- Rule Kind: `VULNERABILITY` +- Affecting Modules: `['Ballerina/os']` + +### Why is this an issue? + +Arguments of system commands are processed by the executed program. The arguments are usually used to configure and influence the behavior of the programs. Control over a single argument might be enough for an attacker to trigger dangerous features like executing arbitrary commands or writing files into specific directories. + +Arguments like -delete or -exec for the find command can alter the expected behavior and result in vulnerabilities: + +### Non-compliant Code Example: + +```ballerina +string terminalPath = ...; +string input = request.getQueryParamValue("input").toString(); +string[] cmd = [..., input]; + +// Sensitive +os:Process result = check os:exec({ + value: terminalPath, + arguments: cmd +}); +``` + +### The solution can be to: + +- Use an allow-list to restrict the arguments to trusted values: + +```ballerina +string terminalPath = ...; +string input = request.getQueryParamValue("input").toString(); +string[] cmd = [..., input]; +string[] allowed = ["main", "main.bal", "bal"]; + +// Compliant +if allowed.some(keyword => keyword.equalsIgnoreCaseAscii(input)) { + os:Process result = check os:exec({ + value: terminalPath, + arguments: cmd + }); +} +``` + +### Additional information + +- [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) +- [CWE - CWE-88 - Argument Injection or Modification](https://cwe.mitre.org/data/definitions/88) +- [CVE-2021-29472 - PHP Supply Chain Attack on Composer](https://blog.sonarsource.com/php-supply-chain-attack-on-composer?_gl=1*8q1b2c*_gcl_au*MTQ0MzcyNzE1Ni4xNzEzNDAwMTc0*_ga*ODk5ODY4NDU1LjE2ODk5Mjg1MjA.*_ga_9JZ0GZ5TC6*MTcxNzkyODY0Mi4yNDQuMS4xNzE3OTM5MjUwLjYwLjAuMA) \ No newline at end of file diff --git a/docs/static-code-analysis-rules/ballerinax-mysql/a-secure-password-should-be-used-when-connecting-to-a-database.md b/docs/static-code-analysis-rules/ballerinax-mysql/a-secure-password-should-be-used-when-connecting-to-a-database.md new file mode 100644 index 00000000..ff34b421 --- /dev/null +++ b/docs/static-code-analysis-rules/ballerinax-mysql/a-secure-password-should-be-used-when-connecting-to-a-database.md @@ -0,0 +1,61 @@ +# Rule definition + +- Rule Title: `A secure password should be used when connecting to a database` +- Rule ID: `undefined` +- Rule Kind: `CODE_SMELL` +- Affecting Modules: `['Ballerinax/mysql']` + +### Why is this an issue? + +When a database lacks authentication requirements, it opens the door for unrestricted access and manipulation of its stored data. Exploiting this vulnerability usually involves identifying the target database and establishing a connection to it without the necessity of any authentication credentials. + +### Non-compliant Code Example: + +```ballerina +configurable string user = ?; +configurable string host = ?; +configurable int port = ?; +configurable string database = ?; + +public function connectToDatabase() { + mysql:Client|error dbClient = new (host = host, + user = user, + password = "", // Noncompliant + port = port, + database = database + ); + + // ... +} +``` + +### The solution can be to: + +- Using a robust password sourced from configurations. The 'password’' property is configured in a configuration file during deployment, and its value should be both strong and unique for each database. + +```ballerina +configurable string user = ?; +configurable string password = ?; // Compliant +configurable string host = ?; +configurable int port = ?; +configurable string database = ?; + +public function connectToDatabase() { + mysql:Client|error dbClient = new (host = host, + user = user, + password = password, // Compliant + port = port, + database = database + ); + + // ... +} +``` + +### Additional information + +- [OWASP - Top 10 2021 Category A2 - Cryptographic Failures](https://owasp.org/Top10/A02_2021-Cryptographic_Failures/) +- [OWASP - Top 10 2021 Category A4 - Insecure Design](https://owasp.org/Top10/A04_2021-Insecure_Design/) +- [OWASP - Top 10 2017 Category A3 - Sensitive Data Exposure](https://owasp.org/www-project-top-ten/2017/A3_2017-Sensitive_Data_Exposure) +- [CWE - CWE-256 - Plaintext Storage of a Password](https://cwe.mitre.org/data/definitions/256) +- [CWE - CWE-916 - Use of Password Hash With Insufficient Computational Effort](https://cwe.mitre.org/data/definitions/916) \ No newline at end of file diff --git a/docs/static-code-analysis-rules/common/passwords-should-not-be-stored-in-plain-text-or-with-a-fast-hashing-algorithm.md b/docs/static-code-analysis-rules/common/passwords-should-not-be-stored-in-plain-text-or-with-a-fast-hashing-algorithm.md new file mode 100644 index 00000000..e5989ad4 --- /dev/null +++ b/docs/static-code-analysis-rules/common/passwords-should-not-be-stored-in-plain-text-or-with-a-fast-hashing-algorithm.md @@ -0,0 +1,108 @@ +# Rule definition + +- Rule Title: `Passwords should not be stored in plain text or with a fast hashing algorithm` +- Rule ID: `undefined` +- Rule Kind: `CODE_SMELL` +- Affecting Modules: `['Ballerinax/mysql', 'Ballerina/postgresql', 'Ballerina/mongodb', 'Ballerina/java.jdbc']` + +### Why is this an issue? + +Attackers who would get access to the stored passwords could reuse them without further attacks or with little additional effort. Obtaining clear-text passwords could make the attackers gain unauthorized access to user accounts, potentially leading to various malicious activities. + +Sensitive code example: + +### Non-compliant Code Example: + +```ballerina +listener http:Listener endpoint = new (8080); +configurable string user= ?; +configurable string password = ?; +configurable string database = ?; + +final jdbc:Client dbClient; + +service / on endpoint { + function init() returns error? { + dbClient = check new jdbc:Client( + url = string `jdbc:h2:./h2/${database}`, + user = user, + password = password + ); + } + + resource function post createUser(http:Request request) returns json|error? { + json data = check request.getJsonPayload(); + string userName = check data.userName; + string password = check data.password; + + // Noncompliant + sql:ExecutionResult result = check dbClient->execute(`INSERT INTO users + VALUES (${userName}, ${password})`); + + // ... + } +} +``` + +Similar sensitive code is applicable across other modules handling database operations. + +### The solution can be to: + +- The hashSha512 hashing function in Ballerina is designed to be secure and resistant to various types of attacks, including brute-force and rainbow table attacks. It is adaptive and allows the implementation of a salt. + +```ballerina +listener http:Listener endpoint = new (8080); +configurable string user= ?; +configurable string password = ?; +configurable string database = ?; + +final jdbc:Client dbClient; + +service / on endpoint { + function init() returns error? { + dbClient = check new jdbc:Client( + url = string `jdbc:h2:./h2/${database}`, + user = user, + password = password + ); + } + + resource function post createUser(http:Request request) returns json|error? { + json data = check request.getJsonPayload(); + string userName = check data.userName; + string password = check data.password; + + // Compliant + // Create a salt + byte[16] salt = [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0]; + foreach int i in 0 ... (salt.length() - 1) { + salt[i] = (check random:createIntInRange(0, 255)); + } + + // Hash the password + byte[] hashedPassword = crypto:hashSha512(password.toBytes(), salt); + + // Add the salt to the hashed password + byte[] saltedHashPassword = [...salt, ...hashedPassword]; + + // convert it to a base 16 string (to save in a DB) + string saltedHashPasswordString = saltedHashPassword.toBase16(); + + // Save to DB + sql:ExecutionResult result = check dbClient->execute(`INSERT INTO users + VALUES (${userName}, ${saltedHashPasswordString})`); + + // ... + } +} +``` + +Similar security measures are applied consistently across other modules handling database operations. + +### Additional information + +- [OWASP - Top 10 2021 Category A2 - Cryptographic Failures](https://owasp.org/Top10/A02_2021-Cryptographic_Failures/) +- [OWASP - Top 10 2021 Category A4 - Insecure Design](https://owasp.org/Top10/A04_2021-Insecure_Design/) +- [OWASP - Top 10 2017 Category A3 - Sensitive Data Exposure](https://owasp.org/www-project-top-ten/2017/A3_2017-Sensitive_Data_Exposure) +- [CWE - CWE-256 - Plaintext Storage of a Password](https://cwe.mitre.org/data/definitions/256) +- [CWE - CWE-916 - Use of Password Hash With Insufficient Computational Effort](https://cwe.mitre.org/data/definitions/916) \ No newline at end of file From 0f42c0df0993d3098483153d08f4e453cbcfcfad Mon Sep 17 00:00:00 2001 From: Tharana Wanigaratne Date: Mon, 17 Jun 2024 09:08:54 +0530 Subject: [PATCH 3/3] Update scan tool docs with steps to add dependency --- .../StaticCodeAnalysisPlatformPlugin.md | 148 +++++++++++++++--- .../StaticCodeAnalyzerCompilerPlugin.md | 65 +++++++- 2 files changed, 188 insertions(+), 25 deletions(-) diff --git a/docs/static-code-analysis-tool/StaticCodeAnalysisPlatformPlugin.md b/docs/static-code-analysis-tool/StaticCodeAnalysisPlatformPlugin.md index 57849ad7..c2554734 100644 --- a/docs/static-code-analysis-tool/StaticCodeAnalysisPlatformPlugin.md +++ b/docs/static-code-analysis-tool/StaticCodeAnalysisPlatformPlugin.md @@ -3,7 +3,13 @@ # Contents - [What is a Static Code Analysis Platform Plugin?](#what-is-a-static-code-analysis-platform-plugin) +- [Components of the Static Code Analysis Platform Plugin](#components-of-the-static-code-analysis-platform-plugin) + - [`StaticCodeAnalysisPlatformPlugin` interface](#staticcodeanalysisplatformplugin-interface) + - [`PlatformPluginContext` interface](#platformplugincontext-interface) - [Steps to create and report analysis results to a platform from a Static Code Analysis Platform Plugin](#steps-to-create-and-report-analysis-results-to-a-platform-from-a-static-code-analysis-platform-plugin) + - [Step 1: Add the static code analysis tool dependencies to the project](#step-1-add-the-static-code-analysis-tool-dependencies-to-the-project) + - [Step 2: Defining the service provider interface file](#step-2-defining-the-service-provider-interface-file) + - [Step 3: Implement the platform plugin](#step-3-implement-the-platform-plugin) - [References](#references) # What is a Static Code Analysis Platform Plugin? @@ -38,35 +44,33 @@ path = "path/to/platform/plugin" Once the user defines `StaticCodeAnalysisPlatformPlugin`'s in the `Scan.toml` file, and executes `bal scan`, the static code analysis tool will first initialize the platform plugins. If additional properties are defined under each platform in the configuration file, they will be passed to the platform plugin during this stage. Then, after performing the core and external static code analysis, the tool will send the analysis results to the platform plugins. The platform plugins will then report the analysis results in a platform-specific format to the platforms. -# Steps to create and report analysis results to a platform from a Static Code Analysis Platform Plugin +# Components of the Static Code Analysis Platform Plugin -1. Implement the `StaticCodeAnalysisPlatformPlugin` interface and build the platform plugin. +The static code analysis tool provides the following components to support reporting analysis results to a platform via platform plugins: -```java -package io.ballerina.scan.platform; +- `StaticCodeAnalysisPlatformPlugin` interface +- `PlatformPluginContext` interface -public class PlatformPlugin implements ScannerPlatformPlugin { - @Override - public String platform() { - return "platformName"; - } +## `StaticCodeAnalysisPlatformPlugin` interface - @Override - public void init(PlatformPluginContext platformPluginContext) { - } +```java +package io.ballerina.scan; - @Override - public void onScan(List issues) { - } +public interface StaticCodeAnalysisPlatformPlugin { + String platform(); + void init(PlatformPluginContext platformArgs); + void onScan(List issues); } ``` -The `ScannerPlatformPlugin` interface has provides the following methods: +The `ScannerPlatformPlugin` interface consists of the following methods: - `platform()` - Returns the name of the platform. - `init(PlatformPluginContext platformPluginContext)` - Initializes the platform plugin with configurations from the `PlatformPluginContext`. - `onScan(List issues)` - Pass analysis results to the platform plugin. +## `PlatformPluginContext` interface + The `PlatformPluginContext` consists of platform-specific configurations that are passed to the platform plugin during initialization. ```java @@ -76,12 +80,118 @@ public interface PlatformPluginContext{ } ``` -It has the following methods: +It consists of the following methods: - `platformArgs()` - Returns in-memory representation of the platform-specific arguments defined in the `Scan.toml` file. - `initiatedByPlatform()` - Returns true if the analysis is triggered by the platform side and false if it is triggered by the static code analysis tool. -2. Define the path to the platform plugin in the `Scan.toml` file. +# Steps to create a Static Code Analysis Platform Plugin + +There are three steps to create a Static Code Analysis Platform Plugin: + +- Add the static code analysis tool dependencies to the project. +- Define the service provider interface file. +- Implementing the platform plugin. + +## Step 1: Add the static code analysis tool dependencies to the project +- Clone the static code analysis tool repository: + +```bash +git clone https://github.com/ballerina-platform/static-code-analysis-tool.git +``` + +- Build the static code analysis tool: + +```bash +cd static-code-analysis-tool +./gradlew clean build +``` + +- Publish the static code analysis tool to the local Maven repository: + +```bash +./gradlew publishToMavenLocal +``` + +- Add the following dependencies to the `build.gradle` file of the new project: + +```groovy +repositories { + mavenLocal() +} + +dependencies { + implementation group: 'io.ballerina.scan', name: 'scan-command', version: '0.1.0' +} +``` + +> Note: Once the static code analysis tool is published to the GitHub package registry, only adding the following dependencies to the `build.gradle` file is sufficient: + +```groovy +repositories { + maven { + maven { + url = 'https://maven.pkg.github.com/ballerina-platform/*' + credentials { + username System.getenv("packageUser") + password System.getenv("packagePAT") + } + } + } +} + +dependencies { + implementation group: 'io.ballerina.scan', name: 'scan-command', version: '0.1.0' +} +``` + +## Step 2: Defining the service provider interface file + +- Create a service file named `io.ballerina.scan.StaticCodeAnalysisPlatformPlugin` in the services' directory. + +``` +📦platform-plugin + ┣ 📂src + ┃ ┗ 📂main + ┃ ┃ ┣ 📂java + ┃ ┃ ┃ ┗ 📂org.arc.platform + ┃ ┃ ┃ ┃ ┗ 📜PlatformPlugin.java + ┃ ┃ ┗ 📂resources + ┃ ┃ ┃ ┗ 📂META-INF + ┃ ┃ ┃ ┃ ┗ 📂services + ┃ ┃ ┃ ┃ ┃ ┗ 📜io.ballerina.scan.StaticCodeAnalysisPlatformPlugin +``` + +- Add the fully qualified name of the platform plugin to the service file. + +```txt +org.arc.platform.PlatformPlugin +``` + +## Step 3: Implement the platform plugin + +- Implement the `StaticCodeAnalysisPlatformPlugin` interface and build the platform plugin. + +```java +package org.arc.platform; + +public class PlatformPlugin implements StaticCodeAnalysisPlatformPlugin { + @Override + public String platform() { + return "platformName"; + } + + @Override + public void init(PlatformPluginContext platformPluginContext) { + } + + @Override + public void onScan(List issues) { + } +} +``` + +- Define the path to the platform plugin in the `Scan.toml` file. ```toml [[platforms]] @@ -91,7 +201,7 @@ path = "path/to/platform/plugin" > Note: The name provided in the `Scan.toml` file should match the platform name returned by the `platform()` method in the platform plugin. -3. Trigger an analysis using the static code analysis tool. +- Trigger an analysis using the static code analysis tool. ```bash bal scan diff --git a/docs/static-code-analysis-tool/StaticCodeAnalyzerCompilerPlugin.md b/docs/static-code-analysis-tool/StaticCodeAnalyzerCompilerPlugin.md index 8db76552..13ef491f 100644 --- a/docs/static-code-analysis-tool/StaticCodeAnalyzerCompilerPlugin.md +++ b/docs/static-code-analysis-tool/StaticCodeAnalyzerCompilerPlugin.md @@ -9,8 +9,9 @@ - [`ScannerContext` interface](#scannercontext-interface) - [`Reporter` interface](#reporter-interface) - [Steps to create a Static Code Analyzer Compiler Plugin](#steps-to-create-a-static-code-analyzer-compiler-plugin) - - [Step 1: Defining the static code analysis rules](#step-1-defining-the-static-code-analysis-rules) - - [Step 2: Implementing the compiler plugin classes to perform static code analysis](#step-2-implementing-the-compiler-plugin-classes-to-perform-static-code-analysis) + - [Step 1: Add the static code analysis tool dependencies to the project](#step-1-add-the-static-code-analysis-tool-dependencies-to-the-project) + - [Step 2: Defining the static code analysis rules](#step-2-defining-the-static-code-analysis-rules) + - [Step 3: Implementing the compiler plugin classes to perform static code analysis](#step-3-implementing-the-compiler-plugin-classes-to-perform-static-code-analysis) - [References](#references) # What is a static code analyzer compiler plugin? @@ -141,7 +142,6 @@ package io.ballerina.scan; public interface Reporter { void reportIssue(Document reportedDocument, Location location, int ruleId); - void reportIssue(Document reportedDocument, Location location, Rule rule); } ``` @@ -190,12 +190,65 @@ It consists of the following types: # Steps to create a Static Code Analyzer Compiler Plugin -There are two steps to follow to create a static code analyzer compiler plugin: +There are three steps to follow to create a static code analyzer compiler plugin: +- Add the static code analysis tool dependencies to the project. - Defining the static code analysis rules. - Implementing the compiler plugin classes to perform static code analysis. -## Step 1: Defining the static code analysis rules +## Step 1: Add the static code analysis tool dependencies to the project +- Clone the static code analysis tool repository: + +```bash +git clone https://github.com/ballerina-platform/static-code-analysis-tool.git +``` + +- Build the static code analysis tool: + +```bash +cd static-code-analysis-tool +./gradlew clean build +``` + +- Publish the static code analysis tool to the local Maven repository: + +```bash +./gradlew publishToMavenLocal +``` + +- Add the following dependencies to the `build.gradle` file of the new project: + +```groovy +repositories { + mavenLocal() +} + +dependencies { + implementation group: 'io.ballerina.scan', name: 'scan-command', version: '0.1.0' +} +``` + +> Note: Once the static code analysis tool is published to the GitHub package registry, only adding the following dependencies to the `build.gradle` file is sufficient: + +```groovy +repositories { + maven { + maven { + url = 'https://maven.pkg.github.com/ballerina-platform/*' + credentials { + username System.getenv("packageUser") + password System.getenv("packagePAT") + } + } + } +} + +dependencies { + implementation group: 'io.ballerina.scan', name: 'scan-command', version: '0.1.0' +} +``` + +## Step 2: Defining the static code analysis rules Create a `rules.json` file in the resources directory with static analysis rules defined in the following format: @@ -228,7 +281,7 @@ Each static code analysis rule consists of three parts: - VULNERABILITY - `description` - Description of the rule. -## Step 2: Implementing the compiler plugin classes to perform static code analysis +## Step 3: Implementing the compiler plugin classes to perform static code analysis - Create a plugin class that extends the `CompilerPlugin` class, and passes the `ScannerContext` retrieved from the `CompilerPluginContext`: