From 37b7aca5f0f5dc5ad56d3c2549e3bbc8d09c7a98 Mon Sep 17 00:00:00 2001 From: SachinAkash01 Date: Mon, 24 Feb 2025 09:30:04 +0530 Subject: [PATCH 01/17] [Automated] Update the native jar versions --- ballerina/Ballerina.toml | 8 ++++---- ballerina/CompilerPlugin.toml | 2 +- ballerina/Dependencies.toml | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/ballerina/Ballerina.toml b/ballerina/Ballerina.toml index 0ee76d9..76ffbfe 100644 --- a/ballerina/Ballerina.toml +++ b/ballerina/Ballerina.toml @@ -1,7 +1,7 @@ [package] org = "ballerina" name = "file" -version = "1.11.0" +version = "1.11.1" authors = ["Ballerina"] keywords = ["file", "path", "directory", "filepath"] repository = "https://github.com/ballerina-platform/module-ballerina-file" @@ -15,12 +15,12 @@ graalvmCompatible = true [[platform.java21.dependency]] groupId = "io.ballerina.stdlib" artifactId = "file-native" -version = "1.11.0" -path = "../native/build/libs/file-native-1.11.0.jar" +version = "1.11.1" +path = "../native/build/libs/file-native-1.11.1-SNAPSHOT.jar" [[platform.java21.dependency]] path = "./lib/org.wso2.transport.local-file-system-6.0.55.jar" [[platform.java21.dependency]] -path = "../test-utils/build/libs/file-test-utils-1.11.0.jar" +path = "../test-utils/build/libs/file-test-utils-1.11.1-SNAPSHOT.jar" scope = "testOnly" diff --git a/ballerina/CompilerPlugin.toml b/ballerina/CompilerPlugin.toml index 79f2941..4f3f102 100644 --- a/ballerina/CompilerPlugin.toml +++ b/ballerina/CompilerPlugin.toml @@ -3,4 +3,4 @@ id = "file-compiler-plugin" class = "io.ballerina.stdlib.file.compiler.FileCompilerPlugin" [[dependency]] -path = "../compiler-plugin/build/libs/file-compiler-plugin-1.11.0.jar" +path = "../compiler-plugin/build/libs/file-compiler-plugin-1.11.1-SNAPSHOT.jar" diff --git a/ballerina/Dependencies.toml b/ballerina/Dependencies.toml index 24fbb59..45d9a49 100644 --- a/ballerina/Dependencies.toml +++ b/ballerina/Dependencies.toml @@ -10,7 +10,7 @@ distribution-version = "2201.11.0" [[package]] org = "ballerina" name = "file" -version = "1.11.0" +version = "1.11.1" dependencies = [ {org = "ballerina", name = "io"}, {org = "ballerina", name = "jballerina.java"}, From 59406f13e35993ea5c4efb66da19464f0a7b2769 Mon Sep 17 00:00:00 2001 From: SachinAkash01 Date: Mon, 24 Feb 2025 12:02:11 +0530 Subject: [PATCH 02/17] Init commit for insecure directory access prevention --- .gitignore | 2 + .../ProcessOutputGobbler.java | 64 +++++++++ .../StaticCodeAnalyzerTest.java | 125 ++++++++++++++++++ .../ballerina_packages/rule1/Ballerina.toml | 8 ++ .../ballerina_packages/rule2/Ballerina.toml | 8 ++ .../src/test/resources/testng.xml | 6 +- compiler-plugin/build.gradle | 4 + .../stdlib/file/compiler/Constants.java | 28 ++++ .../file/compiler/FileCompilerPlugin.java | 11 +- .../FileCodeAnalyzer.java | 11 +- .../compiler/staticcodeanalyzer/FileRule.java | 54 ++++++++ .../FileServiceValidator.java | 3 +- .../InsecureDirectoryAccessAnalyzer.java | 101 ++++++++++++++ .../staticcodeanalyzer/RuleFactory.java | 31 +++++ .../compiler/staticcodeanalyzer/RuleImpl.java | 54 ++++++++ .../src/main/java/module-info.java | 3 + compiler-plugin/src/main/resources/rules.json | 12 ++ gradle.properties | 2 + 18 files changed, 522 insertions(+), 5 deletions(-) create mode 100644 compiler-plugin-test/src/test/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/ProcessOutputGobbler.java create mode 100644 compiler-plugin-test/src/test/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/StaticCodeAnalyzerTest.java create mode 100644 compiler-plugin-test/src/test/resources/static_code_analyzer/ballerina_packages/rule1/Ballerina.toml create mode 100644 compiler-plugin-test/src/test/resources/static_code_analyzer/ballerina_packages/rule2/Ballerina.toml create mode 100644 compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/Constants.java rename compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/{ => staticcodeanalyzer}/FileCodeAnalyzer.java (75%) create mode 100644 compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/FileRule.java rename compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/{ => staticcodeanalyzer}/FileServiceValidator.java (98%) create mode 100644 compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/InsecureDirectoryAccessAnalyzer.java create mode 100644 compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/RuleFactory.java create mode 100644 compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/RuleImpl.java create mode 100644 compiler-plugin/src/main/resources/rules.json diff --git a/.gitignore b/.gitignore index 1707250..9ea4c17 100644 --- a/.gitignore +++ b/.gitignore @@ -41,3 +41,5 @@ target # Ballerina velocity.log* *Ballerina.lock + +compiler-plugin-test/**/target diff --git a/compiler-plugin-test/src/test/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/ProcessOutputGobbler.java b/compiler-plugin-test/src/test/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/ProcessOutputGobbler.java new file mode 100644 index 0000000..54eb2ac --- /dev/null +++ b/compiler-plugin-test/src/test/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/ProcessOutputGobbler.java @@ -0,0 +1,64 @@ +/* + * Copyright (c) 2025, WSO2 LLC. (http://www.wso2.org) + * + * WSO2 LLC. licenses this file to you under the Apache License, + * Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package io.ballerina.stdlib.file.compiler.staticcodeanalyzer; + +import java.io.BufferedReader; +import java.io.IOException; +import java.io.InputStream; +import java.io.InputStreamReader; +import java.nio.charset.StandardCharsets; + +/** + * Helper class to consume the process streams. + */ +class ProcessOutputGobbler implements Runnable { + private final InputStream inputStream; + private final StringBuilder output; + private int exitCode; + + public ProcessOutputGobbler(InputStream inputStream) { + this.inputStream = inputStream; + this.output = new StringBuilder(); + } + + @Override + public void run() { + try (BufferedReader reader = new BufferedReader( + new InputStreamReader(inputStream, StandardCharsets.UTF_8))) { + String line; + while ((line = reader.readLine()) != null) { + output.append(line).append("\n"); + } + } catch (IOException e) { + this.output.append(e.getMessage()); + } + } + + public String getOutput() { + return output.toString(); + } + + public int getExitCode() { + return exitCode; + } + + public void setExitCode(int exitCode) { + this.exitCode = exitCode; + } +} diff --git a/compiler-plugin-test/src/test/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/StaticCodeAnalyzerTest.java b/compiler-plugin-test/src/test/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/StaticCodeAnalyzerTest.java new file mode 100644 index 0000000..ffdd228 --- /dev/null +++ b/compiler-plugin-test/src/test/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/StaticCodeAnalyzerTest.java @@ -0,0 +1,125 @@ +/* + * Copyright (c) 2025, WSO2 LLC. (http://www.wso2.org) + * + * WSO2 LLC. licenses this file to you under the Apache License, + * Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package io.ballerina.stdlib.file.compiler.staticcodeanalyzer; + +import org.testng.Assert; +import org.testng.annotations.BeforeSuite; +import org.testng.annotations.Test; +import org.testng.internal.ExitCode; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.Arrays; +import java.util.Locale; +import java.util.regex.Pattern; +import java.util.stream.Collectors; + +public class StaticCodeAnalyzerTest { + private static final Path RESOURCE_PACKAGES_DIRECTORY = Paths + .get("src", "test", "resources", "static_code_analyzer", "ballerina_packages").toAbsolutePath(); + private static final Path EXPECTED_JSON_OUTPUT_DIRECTORY = Paths + .get("src", "test", "resources", "static_code_analyzer", "expected_output").toAbsolutePath(); + private static final Path BALLERINA_PATH = getBalCommandPath(); + private static final Path JSON_RULES_FILE_PATH = Paths + .get("../", "compiler-plugin", "src", "main", "resources", "rules.json").toAbsolutePath(); + private static final String SCAN_COMMAND = "scan"; + + private static Path getBalCommandPath() { + String balCommand = isWindows() ? "bal.bat" : "bal"; + return Paths.get("../", "target", "ballerina-runtime", "bin", balCommand).toAbsolutePath(); + } + + @BeforeSuite + public void pullScanTool() throws IOException, InterruptedException { + ProcessBuilder processBuilder = new ProcessBuilder(BALLERINA_PATH.toString(), "tool", "pull", SCAN_COMMAND); + ProcessOutputGobbler output = getOutput(processBuilder.start()); + if (Pattern.compile("tool 'scan:.+\\..+\\..+' successfully set as the active version\\.") + .matcher(output.getOutput()).find() || Pattern.compile("tool 'scan:.+\\..+\\..+' is already active\\.") + .matcher(output.getOutput()).find()) { + return; + } + Assert.assertFalse(ExitCode.hasFailure(output.getExitCode())); + } + + @Test + public void validateRulesJson() throws IOException { + String expectedRules = "[" + Arrays.stream(FileRule.values()) + .map(FileRule::toString).collect(Collectors.joining(",")) + "]"; + String actualRules = Files.readString(JSON_RULES_FILE_PATH); + assertJsonEqual(normalizeJson(actualRules), normalizeJson(expectedRules)); + } + + @Test + public void testStaticCodeRules() throws IOException, InterruptedException { + for (FileRule rule : FileRule.values()) { + String targetPackageName = "rule" + rule.getId(); + String actualJsonReport = StaticCodeAnalyzerTest.executeScanProcess(targetPackageName); + String expectedJsonReport = Files + .readString(EXPECTED_JSON_OUTPUT_DIRECTORY.resolve(targetPackageName + ".json")); + assertJsonEqual(actualJsonReport, expectedJsonReport); + } + } + + private static String executeScanProcess(String targetPackage) throws IOException, InterruptedException { + ProcessBuilder processBuilder = new ProcessBuilder(BALLERINA_PATH.toString(), SCAN_COMMAND); + processBuilder.directory(RESOURCE_PACKAGES_DIRECTORY.resolve(targetPackage).toFile()); + ProcessOutputGobbler output = getOutput(processBuilder.start()); + Assert.assertFalse(ExitCode.hasFailure(output.getExitCode())); + return Files.readString(RESOURCE_PACKAGES_DIRECTORY.resolve(targetPackage) + .resolve("target").resolve("report").resolve("scan_results.json")); + } + + private static ProcessOutputGobbler getOutput(Process process) throws InterruptedException { + ProcessOutputGobbler outputGobbler = new ProcessOutputGobbler(process.getInputStream()); + ProcessOutputGobbler errorGobbler = new ProcessOutputGobbler(process.getErrorStream()); + Thread outputThread = new Thread(outputGobbler); + Thread errorThread = new Thread(errorGobbler); + outputThread.start(); + errorThread.start(); + int exitCode = process.waitFor(); + outputGobbler.setExitCode(exitCode); + errorGobbler.setExitCode(exitCode); + outputThread.join(); + errorThread.join(); + return outputGobbler; + } + + private void assertJsonEqual(String actual, String expected) { + Assert.assertEquals(normalizeJson(actual), normalizeJson(expected)); + } + + private static String normalizeJson(String json) { + String normalizedJson = json.replaceAll("\\s*\"\\s*", "\"") + .replaceAll("\\s*:\\s*", ":") + .replaceAll("\\s*,\\s*", ",") + .replaceAll("\\s*\\{\\s*", "{") + .replaceAll("\\s*}\\s*", "}") + .replaceAll("\\s*\\[\\s*", "[") + .replaceAll("\\s*]\\s*", "]") + .replaceAll("\n", "") + .replaceAll(":\".*module-ballerina-file", ":\"module-ballerina-file"); + return isWindows() ? normalizedJson.replaceAll("/", "\\\\\\\\") : normalizedJson; + } + + private static boolean isWindows() { + return System.getProperty("os.name").toLowerCase(Locale.ENGLISH).startsWith("windows"); + } +} diff --git a/compiler-plugin-test/src/test/resources/static_code_analyzer/ballerina_packages/rule1/Ballerina.toml b/compiler-plugin-test/src/test/resources/static_code_analyzer/ballerina_packages/rule1/Ballerina.toml new file mode 100644 index 0000000..80ffe81 --- /dev/null +++ b/compiler-plugin-test/src/test/resources/static_code_analyzer/ballerina_packages/rule1/Ballerina.toml @@ -0,0 +1,8 @@ +[package] +org = "sachink" +name = "rule1" +version = "0.1.0" +distribution = "2201.11.0-20250127-101700-a4b67fe5" + +[build-options] +observabilityIncluded = true diff --git a/compiler-plugin-test/src/test/resources/static_code_analyzer/ballerina_packages/rule2/Ballerina.toml b/compiler-plugin-test/src/test/resources/static_code_analyzer/ballerina_packages/rule2/Ballerina.toml new file mode 100644 index 0000000..979729b --- /dev/null +++ b/compiler-plugin-test/src/test/resources/static_code_analyzer/ballerina_packages/rule2/Ballerina.toml @@ -0,0 +1,8 @@ +[package] +org = "sachink" +name = "rule2" +version = "0.1.0" +distribution = "2201.11.0-20250127-101700-a4b67fe5" + +[build-options] +observabilityIncluded = true diff --git a/compiler-plugin-test/src/test/resources/testng.xml b/compiler-plugin-test/src/test/resources/testng.xml index cbd2756..8af0568 100644 --- a/compiler-plugin-test/src/test/resources/testng.xml +++ b/compiler-plugin-test/src/test/resources/testng.xml @@ -19,10 +19,14 @@ - + + + + + diff --git a/compiler-plugin/build.gradle b/compiler-plugin/build.gradle index dc9efe3..4fd1eae 100644 --- a/compiler-plugin/build.gradle +++ b/compiler-plugin/build.gradle @@ -30,6 +30,8 @@ dependencies { implementation group: 'org.ballerinalang', name: 'ballerina-lang', version: "${ballerinaLangVersion}" implementation group: 'org.ballerinalang', name: 'ballerina-tools-api', version: "${ballerinaLangVersion}" implementation group: 'org.ballerinalang', name: 'ballerina-parser', version: "${ballerinaLangVersion}" + implementation group: 'io.ballerina.scan', name: 'scan-command', version: "${balScanVersion}" + implementation project(":file-native") } def excludePattern = '**/module-info.java' @@ -70,3 +72,5 @@ compileJava { classpath = files() } } + +build.dependsOn ":file-native:build" diff --git a/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/Constants.java b/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/Constants.java new file mode 100644 index 0000000..07029f3 --- /dev/null +++ b/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/Constants.java @@ -0,0 +1,28 @@ +/* + * Copyright (c) 2025, WSO2 LLC. (http://www.wso2.org) + * + * WSO2 LLC. licenses this file to you under the Apache License, + * Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package io.ballerina.stdlib.file.compiler; + +/** + * Constants related to compiler plugin implementation. + */ +public class Constants { + private Constants() {} + + public static final String SCANNER_CONTEXT = "ScannerContext"; +} diff --git a/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/FileCompilerPlugin.java b/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/FileCompilerPlugin.java index da8a208..7bcee6c 100644 --- a/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/FileCompilerPlugin.java +++ b/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/FileCompilerPlugin.java @@ -20,6 +20,10 @@ import io.ballerina.projects.plugins.CompilerPlugin; import io.ballerina.projects.plugins.CompilerPluginContext; +import io.ballerina.scan.ScannerContext; +import io.ballerina.stdlib.file.compiler.staticcodeanalyzer.FileCodeAnalyzer; + +import static io.ballerina.stdlib.file.compiler.Constants.SCANNER_CONTEXT; /** * File compiler plugin. @@ -27,7 +31,10 @@ public class FileCompilerPlugin extends CompilerPlugin { @Override - public void init(CompilerPluginContext compilerPluginContext) { - compilerPluginContext.addCodeAnalyzer(new FileCodeAnalyzer()); + public void init(CompilerPluginContext context) { + Object object = context.userData().get(SCANNER_CONTEXT); + if (object instanceof ScannerContext scannerContext) { + context.addCodeAnalyzer(new FileCodeAnalyzer(scannerContext.getReporter())); + } } } diff --git a/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/FileCodeAnalyzer.java b/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/FileCodeAnalyzer.java similarity index 75% rename from compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/FileCodeAnalyzer.java rename to compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/FileCodeAnalyzer.java index 85bebb3..c00b6ed 100644 --- a/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/FileCodeAnalyzer.java +++ b/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/FileCodeAnalyzer.java @@ -16,18 +16,27 @@ * under the License. */ -package io.ballerina.stdlib.file.compiler; +package io.ballerina.stdlib.file.compiler.staticcodeanalyzer; import io.ballerina.compiler.syntax.tree.SyntaxKind; import io.ballerina.projects.plugins.CodeAnalysisContext; import io.ballerina.projects.plugins.CodeAnalyzer; +import io.ballerina.scan.Reporter; /** * File code analyser. */ public class FileCodeAnalyzer extends CodeAnalyzer { + private final Reporter reporter; + + public FileCodeAnalyzer(Reporter reporter) { + this.reporter = reporter; + } + @Override public void init(CodeAnalysisContext codeAnalysisContext) { codeAnalysisContext.addSyntaxNodeAnalysisTask(new FileServiceValidator(), SyntaxKind.SERVICE_DECLARATION); + codeAnalysisContext.addSyntaxNodeAnalysisTask(new InsecureDirectoryAccessAnalyzer(reporter), + SyntaxKind.METHOD_CALL); } } diff --git a/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/FileRule.java b/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/FileRule.java new file mode 100644 index 0000000..bb20dd5 --- /dev/null +++ b/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/FileRule.java @@ -0,0 +1,54 @@ +/* + * Copyright (c) 2025, WSO2 LLC. (http://www.wso2.org) + * + * WSO2 LLC. licenses this file to you under the Apache License, + * Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package io.ballerina.stdlib.file.compiler.staticcodeanalyzer; + +import io.ballerina.scan.Rule; + +import static io.ballerina.scan.RuleKind.VULNERABILITY; +import static io.ballerina.stdlib.file.compiler.staticcodeanalyzer.RuleFactory.createRule; + +/** + * Represents static code rules specific to the Ballerina File package. + */ +public enum FileRule { + AVOID_INSECURE_DIRECTORY_ACCESS(createRule(1, "Avoid using publicly writable directories for file " + + "operations without proper access controls", VULNERABILITY)), + AVOID_PATH_INJECTION(createRule(2, "I/O function calls should not be vulnerable to path injection " + + "attacks", VULNERABILITY)); + + private final Rule rule; + + FileRule(Rule rule) { + this.rule = rule; + } + + public int getId() { + return this.rule.numericId(); + } + + public Rule getRule() { + return this.rule; + } + + @Override + public String toString() { + return "{\"id\":" + this.getId() + ", \"kind\":\"" + this.rule.kind() + "\"," + + " \"description\" : \"" + this.rule.description() + "\"}"; + } +} diff --git a/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/FileServiceValidator.java b/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/FileServiceValidator.java similarity index 98% rename from compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/FileServiceValidator.java rename to compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/FileServiceValidator.java index 851c3ec..1b9adcd 100644 --- a/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/FileServiceValidator.java +++ b/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/FileServiceValidator.java @@ -16,7 +16,7 @@ * under the License. */ -package io.ballerina.stdlib.file.compiler; +package io.ballerina.stdlib.file.compiler.staticcodeanalyzer; import io.ballerina.compiler.api.symbols.ServiceDeclarationSymbol; import io.ballerina.compiler.api.symbols.Symbol; @@ -33,6 +33,7 @@ import io.ballerina.compiler.syntax.tree.SyntaxKind; import io.ballerina.projects.plugins.AnalysisTask; import io.ballerina.projects.plugins.SyntaxNodeAnalysisContext; +import io.ballerina.stdlib.file.compiler.ErrorCodes; import io.ballerina.tools.diagnostics.Diagnostic; import io.ballerina.tools.diagnostics.DiagnosticFactory; import io.ballerina.tools.diagnostics.DiagnosticInfo; diff --git a/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/InsecureDirectoryAccessAnalyzer.java b/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/InsecureDirectoryAccessAnalyzer.java new file mode 100644 index 0000000..374241a --- /dev/null +++ b/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/InsecureDirectoryAccessAnalyzer.java @@ -0,0 +1,101 @@ +/* + * Copyright (c) 2025, WSO2 LLC. (http://www.wso2.org) + * + * WSO2 LLC. licenses this file to you under the Apache License, + * Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package io.ballerina.stdlib.file.compiler.staticcodeanalyzer; + +import io.ballerina.compiler.syntax.tree.ExpressionNode; +import io.ballerina.compiler.syntax.tree.FunctionArgumentNode; +import io.ballerina.compiler.syntax.tree.FunctionCallExpressionNode; +import io.ballerina.compiler.syntax.tree.PositionalArgumentNode; +import io.ballerina.compiler.syntax.tree.QualifiedNameReferenceNode; +import io.ballerina.compiler.syntax.tree.SeparatedNodeList; +import io.ballerina.projects.Document; +import io.ballerina.projects.plugins.AnalysisTask; +import io.ballerina.projects.plugins.SyntaxNodeAnalysisContext; +import io.ballerina.scan.Reporter; +import io.ballerina.tools.diagnostics.Location; + +import static io.ballerina.stdlib.file.compiler.staticcodeanalyzer.FileRule.AVOID_INSECURE_DIRECTORY_ACCESS; + +public class InsecureDirectoryAccessAnalyzer implements AnalysisTask { + private final Reporter reporter; + + public InsecureDirectoryAccessAnalyzer(Reporter reporter) { + this.reporter = reporter; + } + + @Override + public void perform(SyntaxNodeAnalysisContext context) { + // Check if the current node is a function call + if (!(context.node() instanceof FunctionCallExpressionNode functionCall)) { + return; + } + + // Check if the function call is related to file operations + if (!isFileOperation(functionCall)) { + return; + } + + Document document = getDocument(context); + + // Check if the function call contains insecure directory paths + if (containsInsecureDirectory(functionCall.arguments(), context)) { + // Report a diagnostic for insecure directory access + Location location = functionCall.location(); + this.reporter.reportIssue(document, location, AVOID_INSECURE_DIRECTORY_ACCESS.getId()); + } + } + + public static Document getDocument(SyntaxNodeAnalysisContext context) { + return context.currentPackage().module(context.moduleId()).document(context.documentId()); + } + + private boolean isFileOperation(FunctionCallExpressionNode functionCall) { + if (!(functionCall.functionName() instanceof QualifiedNameReferenceNode qNode)) { + return false; + } + String modulePrefix = qNode.modulePrefix().text(); + String functionName = qNode.identifier().text(); + return modulePrefix.equals("file") && + (functionName.equals("create") || functionName.equals("getAbsolutePath") || + functionName.equals("createTemp") || functionName.equals("createTempDir")); + } + + private boolean containsInsecureDirectory(SeparatedNodeList arguments, + SyntaxNodeAnalysisContext context) { + for (FunctionArgumentNode arg : arguments) { + ExpressionNode expr; + if (arg instanceof PositionalArgumentNode posArg) { + expr = posArg.expression(); + } else { + continue; + } + + // Extract the file path from the expression + String filePath = expr.toSourceCode().trim(); + if (isInsecureDirectory(filePath)) { + return true; + } + } + return false; + } + + private boolean isInsecureDirectory(String filePath) { + return filePath.contains("/tmp") || filePath.contains("TMP") || filePath.contains("TEMP"); + } +} diff --git a/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/RuleFactory.java b/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/RuleFactory.java new file mode 100644 index 0000000..a7af332 --- /dev/null +++ b/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/RuleFactory.java @@ -0,0 +1,31 @@ +/* + * Copyright (c) 2025, WSO2 LLC. (http://www.wso2.org) + * + * WSO2 LLC. licenses this file to you under the Apache License, + * Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package io.ballerina.stdlib.file.compiler.staticcodeanalyzer; + +import io.ballerina.scan.Rule; +import io.ballerina.scan.RuleKind; + +/** + * {@code RuleFactory} contains the logic to create a {@link Rule}. + */ +public class RuleFactory { + public static Rule createRule(int id, String description, RuleKind kind) { + return new RuleImpl(id, description, kind); + } +} diff --git a/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/RuleImpl.java b/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/RuleImpl.java new file mode 100644 index 0000000..e4628b8 --- /dev/null +++ b/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/RuleImpl.java @@ -0,0 +1,54 @@ +/* + * Copyright (c) 2025, WSO2 LLC. (http://www.wso2.org) + * + * WSO2 LLC. licenses this file to you under the Apache License, + * Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package io.ballerina.stdlib.file.compiler.staticcodeanalyzer; + +import io.ballerina.scan.Rule; +import io.ballerina.scan.RuleKind; + +public class RuleImpl implements Rule { + private final int id; + private final String description; + private final RuleKind kind; + + RuleImpl(int id, String description, RuleKind kind) { + this.id = id; + this.description = description; + this.kind = kind; + } + + @Override + public String id() { + return Integer.toString(this.id); + } + + @Override + public int numericId() { + return this.id; + } + + @Override + public String description() { + return this.description; + } + + @Override + public RuleKind kind() { + return this.kind; + } +} diff --git a/compiler-plugin/src/main/java/module-info.java b/compiler-plugin/src/main/java/module-info.java index 8140c1b..4adeb76 100644 --- a/compiler-plugin/src/main/java/module-info.java +++ b/compiler-plugin/src/main/java/module-info.java @@ -17,8 +17,11 @@ */ module io.ballerina.stdlib.file.plugin { + requires io.ballerina.scan; requires io.ballerina.lang; requires io.ballerina.parser; requires io.ballerina.tools.api; + requires java.xml; exports io.ballerina.stdlib.file.compiler; + exports io.ballerina.stdlib.file.compiler.staticcodeanalyzer; } diff --git a/compiler-plugin/src/main/resources/rules.json b/compiler-plugin/src/main/resources/rules.json new file mode 100644 index 0000000..4309bb6 --- /dev/null +++ b/compiler-plugin/src/main/resources/rules.json @@ -0,0 +1,12 @@ +[ + { + "id": 1, + "kind": "VULNERABILITY", + "description": "Avoid using publicly writable directories for file operations without proper access controls" + }, + { + "id": 2, + "kind": "VULNERABILITY", + "description": "I/O function calls should not be vulnerable to path injection attacks" + } +] \ No newline at end of file diff --git a/gradle.properties b/gradle.properties index 8c8b714..192afed 100644 --- a/gradle.properties +++ b/gradle.properties @@ -19,3 +19,5 @@ stdlibLogVersion=2.11.0 stdlibOsVersion=1.9.0 observeVersion=1.4.0 observeInternalVersion=1.4.0 + +balScanVersion=0.5.0 From b24fe8fb92bc5ea0112e9225c86d5ffbe17334c0 Mon Sep 17 00:00:00 2001 From: SachinAkash01 Date: Mon, 24 Feb 2025 12:07:00 +0530 Subject: [PATCH 03/17] Add ballerina test files --- .../static_code_analyzer/ballerina_packages/rule1/main.bal | 5 +++++ .../static_code_analyzer/ballerina_packages/rule2/main.bal | 5 +++++ .../static_code_analyzer/expected_output/rule1.json | 0 .../static_code_analyzer/expected_output/rule2.json | 0 4 files changed, 10 insertions(+) create mode 100644 compiler-plugin-test/src/test/resources/static_code_analyzer/ballerina_packages/rule1/main.bal create mode 100644 compiler-plugin-test/src/test/resources/static_code_analyzer/ballerina_packages/rule2/main.bal create mode 100644 compiler-plugin-test/src/test/resources/static_code_analyzer/expected_output/rule1.json create mode 100644 compiler-plugin-test/src/test/resources/static_code_analyzer/expected_output/rule2.json diff --git a/compiler-plugin-test/src/test/resources/static_code_analyzer/ballerina_packages/rule1/main.bal b/compiler-plugin-test/src/test/resources/static_code_analyzer/ballerina_packages/rule1/main.bal new file mode 100644 index 0000000..31e8785 --- /dev/null +++ b/compiler-plugin-test/src/test/resources/static_code_analyzer/ballerina_packages/rule1/main.bal @@ -0,0 +1,5 @@ +import ballerina/io; + +public function main() { + io:println("Hello, World!"); +} diff --git a/compiler-plugin-test/src/test/resources/static_code_analyzer/ballerina_packages/rule2/main.bal b/compiler-plugin-test/src/test/resources/static_code_analyzer/ballerina_packages/rule2/main.bal new file mode 100644 index 0000000..31e8785 --- /dev/null +++ b/compiler-plugin-test/src/test/resources/static_code_analyzer/ballerina_packages/rule2/main.bal @@ -0,0 +1,5 @@ +import ballerina/io; + +public function main() { + io:println("Hello, World!"); +} diff --git a/compiler-plugin-test/src/test/resources/static_code_analyzer/expected_output/rule1.json b/compiler-plugin-test/src/test/resources/static_code_analyzer/expected_output/rule1.json new file mode 100644 index 0000000..e69de29 diff --git a/compiler-plugin-test/src/test/resources/static_code_analyzer/expected_output/rule2.json b/compiler-plugin-test/src/test/resources/static_code_analyzer/expected_output/rule2.json new file mode 100644 index 0000000..e69de29 From 217da554ad6ad8f8d1107ac4650518361aee27ca Mon Sep 17 00:00:00 2001 From: SachinAkash01 Date: Mon, 24 Feb 2025 12:08:12 +0530 Subject: [PATCH 04/17] Fix checkstyle issues --- compiler-plugin/src/main/resources/rules.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler-plugin/src/main/resources/rules.json b/compiler-plugin/src/main/resources/rules.json index 4309bb6..74f2815 100644 --- a/compiler-plugin/src/main/resources/rules.json +++ b/compiler-plugin/src/main/resources/rules.json @@ -9,4 +9,4 @@ "kind": "VULNERABILITY", "description": "I/O function calls should not be vulnerable to path injection attacks" } -] \ No newline at end of file +] From df440bd7a16b14a2f71c8dfd3043174fcb5ac7f0 Mon Sep 17 00:00:00 2001 From: SachinAkash01 Date: Mon, 24 Feb 2025 12:14:35 +0530 Subject: [PATCH 05/17] Update changelog --- changelog.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/changelog.md b/changelog.md index ba038d3..a23f50e 100644 --- a/changelog.md +++ b/changelog.md @@ -6,6 +6,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added +- [Add static code rules](https://github.com/ballerina-platform/ballerina-library/issues/7283) + + ### Changed - [Change the listener configuration as an included parameter](https://github.com/ballerina-platform/ballerina-library/issues/7494) From 1119c8303796060d9ac977d88593e7c2add849df Mon Sep 17 00:00:00 2001 From: SachinAkash01 Date: Mon, 24 Feb 2025 17:46:08 +0530 Subject: [PATCH 06/17] Add insecure directory access analyzer --- .../ballerina_packages/rule2/main.bal | 27 +++++- .../expected_output/rule1.json | 22 +++++ .../stdlib/file/compiler/Constants.java | 4 + .../staticcodeanalyzer/FileCodeAnalyzer.java | 2 +- .../InsecureDirectoryAccessAnalyzer.java | 84 +++++++++++-------- 5 files changed, 103 insertions(+), 36 deletions(-) diff --git a/compiler-plugin-test/src/test/resources/static_code_analyzer/ballerina_packages/rule2/main.bal b/compiler-plugin-test/src/test/resources/static_code_analyzer/ballerina_packages/rule2/main.bal index 31e8785..0b7e47f 100644 --- a/compiler-plugin-test/src/test/resources/static_code_analyzer/ballerina_packages/rule2/main.bal +++ b/compiler-plugin-test/src/test/resources/static_code_analyzer/ballerina_packages/rule2/main.bal @@ -1,5 +1,30 @@ +// Copyright (c) 2025 WSO2 LLC. (http://www.wso2.org) +// +// WSO2 LLC. licenses this file to you under the Apache License, +// Version 2.0 (the "License"); you may not use this file except +// in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +import ballerina/file; +import ballerina/os; import ballerina/io; public function main() { - io:println("Hello, World!"); + string tempFolderPath = os:getEnv("TMP"); + error? output = file:create(tempFolderPath + "/" + "myfile.txt"); + + if (output is error) { + io:println("Error occurred: " + output.message()); + } else { + io:println("File created successfully: " + tempFolderPath + "/" + "myfile.txt"); + } } diff --git a/compiler-plugin-test/src/test/resources/static_code_analyzer/expected_output/rule1.json b/compiler-plugin-test/src/test/resources/static_code_analyzer/expected_output/rule1.json index e69de29..8519b23 100644 --- a/compiler-plugin-test/src/test/resources/static_code_analyzer/expected_output/rule1.json +++ b/compiler-plugin-test/src/test/resources/static_code_analyzer/expected_output/rule1.json @@ -0,0 +1,22 @@ +[ + { + "location": { + "filePath": "main.bal", + "startLine": 5, + "endLine": 5, + "startColumn": 28, + "endColumn": 44, + "startOffset": 119, + "length": 16 + }, + "rule": { + "id": "ballerina/file:1", + "numericId": 1, + "description": "Avoid using publicly writable directories for file operations without proper access controls", + "ruleKind": "VULNERABILITY" + }, + "source": "BUILT_IN", + "fileName": "os_test/main.bal", + "filePath": "/Users/sachink/Desktop/os-test/main.bal" + } +] diff --git a/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/Constants.java b/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/Constants.java index 07029f3..dbdd927 100644 --- a/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/Constants.java +++ b/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/Constants.java @@ -25,4 +25,8 @@ public class Constants { private Constants() {} public static final String SCANNER_CONTEXT = "ScannerContext"; + public static final String OS = "os"; + public static final String GET_ENV = "getEnv"; + public static final String FILE = "file"; + public static final String FILE_CREATE = "create"; } diff --git a/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/FileCodeAnalyzer.java b/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/FileCodeAnalyzer.java index c00b6ed..52eb32e 100644 --- a/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/FileCodeAnalyzer.java +++ b/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/FileCodeAnalyzer.java @@ -37,6 +37,6 @@ public FileCodeAnalyzer(Reporter reporter) { public void init(CodeAnalysisContext codeAnalysisContext) { codeAnalysisContext.addSyntaxNodeAnalysisTask(new FileServiceValidator(), SyntaxKind.SERVICE_DECLARATION); codeAnalysisContext.addSyntaxNodeAnalysisTask(new InsecureDirectoryAccessAnalyzer(reporter), - SyntaxKind.METHOD_CALL); + SyntaxKind.FUNCTION_CALL); } } diff --git a/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/InsecureDirectoryAccessAnalyzer.java b/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/InsecureDirectoryAccessAnalyzer.java index 374241a..9447e0a 100644 --- a/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/InsecureDirectoryAccessAnalyzer.java +++ b/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/InsecureDirectoryAccessAnalyzer.java @@ -18,9 +18,11 @@ package io.ballerina.stdlib.file.compiler.staticcodeanalyzer; +import io.ballerina.compiler.syntax.tree.BasicLiteralNode; import io.ballerina.compiler.syntax.tree.ExpressionNode; import io.ballerina.compiler.syntax.tree.FunctionArgumentNode; import io.ballerina.compiler.syntax.tree.FunctionCallExpressionNode; +import io.ballerina.compiler.syntax.tree.Node; import io.ballerina.compiler.syntax.tree.PositionalArgumentNode; import io.ballerina.compiler.syntax.tree.QualifiedNameReferenceNode; import io.ballerina.compiler.syntax.tree.SeparatedNodeList; @@ -29,7 +31,12 @@ import io.ballerina.projects.plugins.SyntaxNodeAnalysisContext; import io.ballerina.scan.Reporter; import io.ballerina.tools.diagnostics.Location; +import org.ballerinalang.model.tree.expressions.StringTemplateLiteralNode; +import static io.ballerina.stdlib.file.compiler.Constants.OS; +import static io.ballerina.stdlib.file.compiler.Constants.GET_ENV; +import static io.ballerina.stdlib.file.compiler.Constants.FILE; +import static io.ballerina.stdlib.file.compiler.Constants.FILE_CREATE; import static io.ballerina.stdlib.file.compiler.staticcodeanalyzer.FileRule.AVOID_INSECURE_DIRECTORY_ACCESS; public class InsecureDirectoryAccessAnalyzer implements AnalysisTask { @@ -41,56 +48,65 @@ public InsecureDirectoryAccessAnalyzer(Reporter reporter) { @Override public void perform(SyntaxNodeAnalysisContext context) { - // Check if the current node is a function call if (!(context.node() instanceof FunctionCallExpressionNode functionCall)) { return; } - // Check if the function call is related to file operations - if (!isFileOperation(functionCall)) { + if (!isFileMethodCall(functionCall)) { return; } Document document = getDocument(context); - - // Check if the function call contains insecure directory paths - if (containsInsecureDirectory(functionCall.arguments(), context)) { - // Report a diagnostic for insecure directory access - Location location = functionCall.location(); - this.reporter.reportIssue(document, location, AVOID_INSECURE_DIRECTORY_ACCESS.getId()); - } + Location location = functionCall.location(); + this.reporter.reportIssue(document, location, AVOID_INSECURE_DIRECTORY_ACCESS.getId()); } public static Document getDocument(SyntaxNodeAnalysisContext context) { return context.currentPackage().module(context.moduleId()).document(context.documentId()); } - private boolean isFileOperation(FunctionCallExpressionNode functionCall) { - if (!(functionCall.functionName() instanceof QualifiedNameReferenceNode qNode)) { - return false; - } - String modulePrefix = qNode.modulePrefix().text(); - String functionName = qNode.identifier().text(); - return modulePrefix.equals("file") && - (functionName.equals("create") || functionName.equals("getAbsolutePath") || - functionName.equals("createTemp") || functionName.equals("createTempDir")); - } - - private boolean containsInsecureDirectory(SeparatedNodeList arguments, - SyntaxNodeAnalysisContext context) { - for (FunctionArgumentNode arg : arguments) { - ExpressionNode expr; - if (arg instanceof PositionalArgumentNode posArg) { - expr = posArg.expression(); - } else { - continue; - } + private boolean isFileMethodCall(FunctionCallExpressionNode functionCall) { + Node currentNode = functionCall; + while (currentNode != null) { + if (currentNode instanceof FunctionCallExpressionNode currentMethodCall) { + if (currentMethodCall.functionName() instanceof QualifiedNameReferenceNode qNode) { + String modulePrefix = qNode.modulePrefix().text(); + String functionName = qNode.identifier().text(); + if (modulePrefix.equals(OS) && functionName.equals(GET_ENV)) { + String envVarName = currentMethodCall.arguments().get(0).toString(); + if (envVarName.equals("\"TMP\"") || envVarName.equals("\"TEMP\"") || + envVarName.equals("\"TMPDIR\"")) { + return true; + } + } + } - // Extract the file path from the expression - String filePath = expr.toSourceCode().trim(); - if (isInsecureDirectory(filePath)) { - return true; + if (currentMethodCall.functionName() instanceof QualifiedNameReferenceNode fileCallNode) { + String fileModulePrefix = fileCallNode.modulePrefix().text(); + String fileFunctionName = fileCallNode.identifier().text(); + if (fileModulePrefix.equals(FILE) && fileFunctionName.equals(FILE_CREATE)) { + SeparatedNodeList arguments = currentMethodCall.arguments(); + if (arguments != null && !arguments.isEmpty()) { + FunctionArgumentNode pathArg = arguments.get(0); + if (pathArg instanceof PositionalArgumentNode posArg) { + ExpressionNode pathExpr = posArg.expression(); + if (pathExpr instanceof BasicLiteralNode pathLiteral) { + String filePath = pathLiteral.toString().trim(); + if (isInsecureDirectory(filePath)) { + return true; + } + } else if (pathExpr instanceof StringTemplateLiteralNode templatePath) { + String filePath = templatePath.toString().trim(); + if (isInsecureDirectory(filePath)) { + return true; + } + } + } + } + } + } } + currentNode = currentNode.parent(); } return false; } From 0fadc0a27013a5760539b9f557a5321cb4cd0bbd Mon Sep 17 00:00:00 2001 From: SachinAkash01 Date: Mon, 24 Feb 2025 18:31:53 +0530 Subject: [PATCH 07/17] Update rule1.json --- .../ballerina_packages/rule1/main.bal | 27 ++++++- .../ballerina_packages/rule2/main.bal | 11 +-- .../expected_output/rule1.json | 80 ++++++++++++++----- 3 files changed, 85 insertions(+), 33 deletions(-) diff --git a/compiler-plugin-test/src/test/resources/static_code_analyzer/ballerina_packages/rule1/main.bal b/compiler-plugin-test/src/test/resources/static_code_analyzer/ballerina_packages/rule1/main.bal index 31e8785..0b7e47f 100644 --- a/compiler-plugin-test/src/test/resources/static_code_analyzer/ballerina_packages/rule1/main.bal +++ b/compiler-plugin-test/src/test/resources/static_code_analyzer/ballerina_packages/rule1/main.bal @@ -1,5 +1,30 @@ +// Copyright (c) 2025 WSO2 LLC. (http://www.wso2.org) +// +// WSO2 LLC. licenses this file to you under the Apache License, +// Version 2.0 (the "License"); you may not use this file except +// in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +import ballerina/file; +import ballerina/os; import ballerina/io; public function main() { - io:println("Hello, World!"); + string tempFolderPath = os:getEnv("TMP"); + error? output = file:create(tempFolderPath + "/" + "myfile.txt"); + + if (output is error) { + io:println("Error occurred: " + output.message()); + } else { + io:println("File created successfully: " + tempFolderPath + "/" + "myfile.txt"); + } } diff --git a/compiler-plugin-test/src/test/resources/static_code_analyzer/ballerina_packages/rule2/main.bal b/compiler-plugin-test/src/test/resources/static_code_analyzer/ballerina_packages/rule2/main.bal index 0b7e47f..f886d12 100644 --- a/compiler-plugin-test/src/test/resources/static_code_analyzer/ballerina_packages/rule2/main.bal +++ b/compiler-plugin-test/src/test/resources/static_code_analyzer/ballerina_packages/rule2/main.bal @@ -14,17 +14,8 @@ // specific language governing permissions and limitations // under the License. -import ballerina/file; -import ballerina/os; import ballerina/io; public function main() { - string tempFolderPath = os:getEnv("TMP"); - error? output = file:create(tempFolderPath + "/" + "myfile.txt"); - - if (output is error) { - io:println("Error occurred: " + output.message()); - } else { - io:println("File created successfully: " + tempFolderPath + "/" + "myfile.txt"); - } + io:println("Hello, World!"); } diff --git a/compiler-plugin-test/src/test/resources/static_code_analyzer/expected_output/rule1.json b/compiler-plugin-test/src/test/resources/static_code_analyzer/expected_output/rule1.json index 8519b23..3025f77 100644 --- a/compiler-plugin-test/src/test/resources/static_code_analyzer/expected_output/rule1.json +++ b/compiler-plugin-test/src/test/resources/static_code_analyzer/expected_output/rule1.json @@ -1,22 +1,58 @@ -[ - { - "location": { - "filePath": "main.bal", - "startLine": 5, - "endLine": 5, - "startColumn": 28, - "endColumn": 44, - "startOffset": 119, - "length": 16 - }, - "rule": { - "id": "ballerina/file:1", - "numericId": 1, - "description": "Avoid using publicly writable directories for file operations without proper access controls", - "ruleKind": "VULNERABILITY" - }, - "source": "BUILT_IN", - "fileName": "os_test/main.bal", - "filePath": "/Users/sachink/Desktop/os-test/main.bal" - } -] +[ { + "location" : { + "filePath" : "main.bal", + "startLine" : 22, + "endLine" : 22, + "startColumn" : 20, + "endColumn" : 68, + "startOffset" : 800, + "length" : 48 + }, + "rule" : { + "id" : "ballerina/file:1", + "numericId" : 1, + "description" : "Avoid using publicly writable directories for file operations without proper access controls", + "ruleKind" : "VULNERABILITY" + }, + "source" : "BUILT_IN", + "fileName" : "rule1/main.bal", + "filePath" : "/Users/sachink/Desktop/module-ballerina-file/compiler-plugin-test/src/test/resources/static_code_analyzer/ballerina_packages/rule1/main.bal" +}, { + "location" : { + "filePath" : "main.bal", + "startLine" : 25, + "endLine" : 25, + "startColumn" : 8, + "endColumn" : 57, + "startOffset" : 886, + "length" : 49 + }, + "rule" : { + "id" : "ballerina/file:1", + "numericId" : 1, + "description" : "Avoid using publicly writable directories for file operations without proper access controls", + "ruleKind" : "VULNERABILITY" + }, + "source" : "BUILT_IN", + "fileName" : "rule1/main.bal", + "filePath" : "/Users/sachink/Desktop/module-ballerina-file/compiler-plugin-test/src/test/resources/static_code_analyzer/ballerina_packages/rule1/main.bal" +}, { + "location" : { + "filePath" : "main.bal", + "startLine" : 27, + "endLine" : 27, + "startColumn" : 8, + "endColumn" : 87, + "startOffset" : 958, + "length" : 79 + }, + "rule" : { + "id" : "ballerina/file:1", + "numericId" : 1, + "description" : "Avoid using publicly writable directories for file operations without proper access controls", + "ruleKind" : "VULNERABILITY" + }, + "source" : "BUILT_IN", + "fileName" : "rule1/main.bal", + "filePath" : "/Users/sachink/Desktop/module-ballerina-file/compiler-plugin-test/src/test/resources/static_code_analyzer/ballerina_packages/rule1/main.bal" +} ] From 9142c37ca330f7f9e85c12c31c0daf60974c7e90 Mon Sep 17 00:00:00 2001 From: SachinAkash01 Date: Mon, 24 Feb 2025 20:34:35 +0530 Subject: [PATCH 08/17] Add file path injection analyzer --- .../ballerina_packages/rule2/main.bal | 11 +- .../expected_output/rule2.json | 20 ++ .../staticcodeanalyzer/FileCodeAnalyzer.java | 2 + .../FilePathInjectionAnalyzer.java | 235 ++++++++++++++++++ 4 files changed, 267 insertions(+), 1 deletion(-) create mode 100644 compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/FilePathInjectionAnalyzer.java diff --git a/compiler-plugin-test/src/test/resources/static_code_analyzer/ballerina_packages/rule2/main.bal b/compiler-plugin-test/src/test/resources/static_code_analyzer/ballerina_packages/rule2/main.bal index f886d12..5379270 100644 --- a/compiler-plugin-test/src/test/resources/static_code_analyzer/ballerina_packages/rule2/main.bal +++ b/compiler-plugin-test/src/test/resources/static_code_analyzer/ballerina_packages/rule2/main.bal @@ -14,8 +14,17 @@ // specific language governing permissions and limitations // under the License. +import ballerina/file; import ballerina/io; +public function executeCommand(string fileName) returns file:Error? { + string unsafeFilePath = "./target/" + fileName; + file:Error? remove = file:remove(unsafeFilePath); + + return remove; +} + public function main() { - io:println("Hello, World!"); + file:Error? result = executeCommand("sample.json"); + io:println("Result: ", result); } diff --git a/compiler-plugin-test/src/test/resources/static_code_analyzer/expected_output/rule2.json b/compiler-plugin-test/src/test/resources/static_code_analyzer/expected_output/rule2.json index e69de29..5df553c 100644 --- a/compiler-plugin-test/src/test/resources/static_code_analyzer/expected_output/rule2.json +++ b/compiler-plugin-test/src/test/resources/static_code_analyzer/expected_output/rule2.json @@ -0,0 +1,20 @@ +[ { + "location" : { + "filePath" : "main.bal", + "startLine" : 21, + "endLine" : 21, + "startColumn" : 28, + "endColumn" : 44, + "startOffset" : 762, + "length" : 16 + }, + "rule" : { + "id" : "ballerina/file:2", + "numericId" : 2, + "description" : "I/O function calls should not be vulnerable to path injection attacks", + "ruleKind" : "VULNERABILITY" + }, + "source" : "BUILT_IN", + "fileName" : "rule2/main.bal", + "filePath" : "/Users/sachink/Desktop/module-ballerina-file/compiler-plugin-test/src/test/resources/static_code_analyzer/ballerina_packages/rule2/main.bal" +} ] diff --git a/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/FileCodeAnalyzer.java b/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/FileCodeAnalyzer.java index 52eb32e..b6565d9 100644 --- a/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/FileCodeAnalyzer.java +++ b/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/FileCodeAnalyzer.java @@ -38,5 +38,7 @@ public void init(CodeAnalysisContext codeAnalysisContext) { codeAnalysisContext.addSyntaxNodeAnalysisTask(new FileServiceValidator(), SyntaxKind.SERVICE_DECLARATION); codeAnalysisContext.addSyntaxNodeAnalysisTask(new InsecureDirectoryAccessAnalyzer(reporter), SyntaxKind.FUNCTION_CALL); + codeAnalysisContext.addSyntaxNodeAnalysisTask(new FilePathInjectionAnalyzer(reporter), + SyntaxKind.FUNCTION_CALL); } } diff --git a/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/FilePathInjectionAnalyzer.java b/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/FilePathInjectionAnalyzer.java new file mode 100644 index 0000000..4a34439 --- /dev/null +++ b/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/FilePathInjectionAnalyzer.java @@ -0,0 +1,235 @@ +/* + * Copyright (c) 2025, WSO2 LLC. (http://www.wso2.org) + * + * WSO2 LLC. licenses this file to you under the Apache License, + * Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package io.ballerina.stdlib.file.compiler.staticcodeanalyzer; + +import io.ballerina.compiler.syntax.tree.BinaryExpressionNode; +import io.ballerina.compiler.syntax.tree.CaptureBindingPatternNode; +import io.ballerina.compiler.syntax.tree.ExpressionNode; +import io.ballerina.compiler.syntax.tree.FunctionArgumentNode; +import io.ballerina.compiler.syntax.tree.FunctionBodyBlockNode; +import io.ballerina.compiler.syntax.tree.FunctionCallExpressionNode; +import io.ballerina.compiler.syntax.tree.FunctionDefinitionNode; +import io.ballerina.compiler.syntax.tree.Node; +import io.ballerina.compiler.syntax.tree.NodeList; +import io.ballerina.compiler.syntax.tree.ParameterNode; +import io.ballerina.compiler.syntax.tree.PositionalArgumentNode; +import io.ballerina.compiler.syntax.tree.RequiredParameterNode; +import io.ballerina.compiler.syntax.tree.SimpleNameReferenceNode; +import io.ballerina.compiler.syntax.tree.StatementNode; +import io.ballerina.compiler.syntax.tree.SyntaxKind; +import io.ballerina.compiler.syntax.tree.VariableDeclarationNode; +import io.ballerina.projects.Document; +import io.ballerina.projects.plugins.AnalysisTask; +import io.ballerina.projects.plugins.SyntaxNodeAnalysisContext; +import io.ballerina.scan.Reporter; +import io.ballerina.tools.diagnostics.Location; + + +import static io.ballerina.stdlib.file.compiler.staticcodeanalyzer.FileRule.AVOID_PATH_INJECTION; + +/** + * Analyzer to detect potential file path injection vulnerabilities. + */ +public class FilePathInjectionAnalyzer implements AnalysisTask { + + private final Reporter reporter; + + public FilePathInjectionAnalyzer(Reporter reporter) { + this.reporter = reporter; + } + + @Override + public void perform(SyntaxNodeAnalysisContext context) { + if (!(context.node() instanceof FunctionCallExpressionNode functionCall)) { + return; + } + + Document document = getDocument(context); + + String functionName = functionCall.functionName().toString(); + + // Detect vulnerable file function calls + if ("file:remove".equals(functionName) || "file:read".equals(functionName) || + "file:write".equals(functionName)) { + if (!isSafePath(functionCall, context)) { + Location location = functionCall.location(); + this.reporter.reportIssue(document, location, AVOID_PATH_INJECTION.getId()); + } + } + } + + public static Document getDocument(SyntaxNodeAnalysisContext context) { + return context.currentPackage().module(context.moduleId()).document(context.documentId()); + } + + private boolean isSafePath(FunctionCallExpressionNode functionCall, SyntaxNodeAnalysisContext context) { + NodeList arguments = functionCall.arguments(); + if (arguments.isEmpty()) { + return true; + } + + FunctionArgumentNode firstArg = arguments.get(0); + ExpressionNode argument; + + if (firstArg instanceof PositionalArgumentNode) { + argument = ((PositionalArgumentNode) firstArg).expression(); + } else { + return true; + } + + // Direct concatenation detection + if (argument instanceof BinaryExpressionNode binaryExpression) { + if (binaryExpression.operator().kind() == SyntaxKind.PLUS_TOKEN) { + return false; // Unsafe concatenation detected + } + } + + // Variable reference detection + if (argument instanceof SimpleNameReferenceNode variableRef) { + return isVariableSafe(variableRef, context); + } + return true; + } + + private boolean isVariableSafe(SimpleNameReferenceNode variableRef, SyntaxNodeAnalysisContext context) { + String variableName = variableRef.name().text(); + Node currentNode = variableRef.parent(); + while (currentNode != null) { + if (currentNode instanceof FunctionBodyBlockNode functionBody) { + for (StatementNode statement : functionBody.statements()) { + if (statement instanceof VariableDeclarationNode varDecl) { + if (varDecl.typedBindingPattern().bindingPattern() instanceof + CaptureBindingPatternNode bindingPattern) { + if (bindingPattern.variableName().text().equals(variableName)) { + // Check if assigned using concatenation + if (varDecl.initializer().orElse(null) instanceof BinaryExpressionNode) { + BinaryExpressionNode binaryExpr = (BinaryExpressionNode) + varDecl.initializer().get(); + if (binaryExpr.operator().kind() == SyntaxKind.PLUS_TOKEN) { + return isFunctionParameter(variableRef); + } + } + // Check if assigned directly from a function parameter + if (varDecl.initializer().orElse(null) instanceof SimpleNameReferenceNode) { + SimpleNameReferenceNode initializer = (SimpleNameReferenceNode) + varDecl.initializer().get(); + return isFunctionParameter(initializer); + } + return true; + } + } + } + } + } + currentNode = currentNode.parent(); + } + return true; + } + + private boolean isFunctionParameter(SimpleNameReferenceNode variableRef) { + String paramName = variableRef.name().text(); + Node currentNode = variableRef.parent(); + while (currentNode != null) { + if (currentNode instanceof FunctionDefinitionNode functionDef) { + // Check if function is public + boolean isPublic = functionDef.qualifierList().stream() + .anyMatch(q -> q.kind() == SyntaxKind.PUBLIC_KEYWORD); + + // Iterate over function parameters to check direct reference + for (ParameterNode param : functionDef.functionSignature().parameters()) { + if (param instanceof RequiredParameterNode reqParam) { + // Check direct parameter reference + if (reqParam.paramName().isPresent() && + reqParam.paramName().get().text().equals(paramName)) { + return !isPublic; + } + // Check indirect reference chain (assignments) + if (isIndirectFunctionParameter(variableRef, reqParam)) { + return !isPublic; + } + } + } + } + currentNode = currentNode.parent(); + } + return true; + } + + /** + * Checks if the variable is indirectly referencing a function parameter + * by tracing back the assignment chain. + */ + private boolean isIndirectFunctionParameter(SimpleNameReferenceNode variableRef, RequiredParameterNode reqParam) { + Node currentNode = variableRef.parent(); + while (currentNode != null) { + if (currentNode instanceof FunctionBodyBlockNode functionBody) { + for (StatementNode statement : functionBody.statements()) { + if (statement instanceof VariableDeclarationNode varDecl) { + if (varDecl.typedBindingPattern().bindingPattern() instanceof + CaptureBindingPatternNode bindingPattern) { + if (bindingPattern.variableName().text().equals(variableRef.name().text())) { + // Now check if this variable is assigned to another variable + if (varDecl.initializer().isPresent()) { + ExpressionNode initializer = varDecl.initializer().get(); + // If it's a reference to the function parameter, return true + if (initializer instanceof SimpleNameReferenceNode initializerRef) { + if (initializerRef.name().text().equals(reqParam.paramName().get().text())) { + return true; + } + } + // If it's a binary expression (like concatenation), recurse + if (initializer instanceof BinaryExpressionNode binaryExpr) { + if (binaryExpr.operator().kind() == SyntaxKind.PLUS_TOKEN) { + // Recursively check both sides of the binary expression + if (isIndirectFunctionParameterFromBinary(binaryExpr, reqParam)) { + return true; + } + } + } + } + } + } + } + } + } + currentNode = currentNode.parent(); + } + return false; + } + + /** + * Recursively checks both sides of the binary expression (e.g., concatenation) for a reference to the + * function parameter. + */ + private boolean isIndirectFunctionParameterFromBinary(BinaryExpressionNode binaryExpr, + RequiredParameterNode reqParam) { + // Check both the left and right sides of the binary expression + if (binaryExpr.lhsExpr() instanceof SimpleNameReferenceNode leftRef) { + if (leftRef.name().text().equals(reqParam.paramName().get().text())) { + return true; + } + } + if (binaryExpr.rhsExpr() instanceof SimpleNameReferenceNode rightRef) { + if (rightRef.name().text().equals(reqParam.paramName().get().text())) { + return true; + } + } + return false; + } +} From 49d3a094ba6de6d00c7c6807a1a2a32d4842cf07 Mon Sep 17 00:00:00 2001 From: SachinAkash01 Date: Mon, 24 Feb 2025 20:53:01 +0530 Subject: [PATCH 09/17] Add Constants --- .../java/io/ballerina/stdlib/file/compiler/Constants.java | 3 +++ .../staticcodeanalyzer/FilePathInjectionAnalyzer.java | 8 +++++--- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/Constants.java b/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/Constants.java index dbdd927..9de8c28 100644 --- a/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/Constants.java +++ b/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/Constants.java @@ -29,4 +29,7 @@ private Constants() {} public static final String GET_ENV = "getEnv"; public static final String FILE = "file"; public static final String FILE_CREATE = "create"; + public static final String FILE_REMOVE = "file:remove"; + public static final String FILE_READ = "file:read"; + public static final String FILE_WRITE = "file:write"; } diff --git a/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/FilePathInjectionAnalyzer.java b/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/FilePathInjectionAnalyzer.java index 4a34439..b5ff1de 100644 --- a/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/FilePathInjectionAnalyzer.java +++ b/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/FilePathInjectionAnalyzer.java @@ -40,7 +40,9 @@ import io.ballerina.scan.Reporter; import io.ballerina.tools.diagnostics.Location; - +import static io.ballerina.stdlib.file.compiler.Constants.FILE_REMOVE; +import static io.ballerina.stdlib.file.compiler.Constants.FILE_READ; +import static io.ballerina.stdlib.file.compiler.Constants.FILE_WRITE; import static io.ballerina.stdlib.file.compiler.staticcodeanalyzer.FileRule.AVOID_PATH_INJECTION; /** @@ -65,8 +67,8 @@ public void perform(SyntaxNodeAnalysisContext context) { String functionName = functionCall.functionName().toString(); // Detect vulnerable file function calls - if ("file:remove".equals(functionName) || "file:read".equals(functionName) || - "file:write".equals(functionName)) { + if (FILE_REMOVE.equals(functionName) || FILE_READ.equals(functionName) || + FILE_WRITE.equals(functionName)) { if (!isSafePath(functionCall, context)) { Location location = functionCall.location(); this.reporter.reportIssue(document, location, AVOID_PATH_INJECTION.getId()); From f8dca8a38573283ce5e8c5f13634982cdb9ae818 Mon Sep 17 00:00:00 2001 From: SachinAkash01 Date: Thu, 27 Feb 2025 15:04:34 +0530 Subject: [PATCH 10/17] Fix test failures --- .../file/compiler/FileCodeAnalyzer.java | 34 +++++++++++++++++++ .../file/compiler/FileCompilerPlugin.java | 5 +-- .../FilePathInjectionAnalyzer.java | 16 +++------ ...lyzer.java => FileStaticCodeAnalyzer.java} | 11 +++--- .../staticcodeanalyzer/RuleFactory.java | 4 +++ 5 files changed, 51 insertions(+), 19 deletions(-) create mode 100644 compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/FileCodeAnalyzer.java rename compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/{FileCodeAnalyzer.java => FileStaticCodeAnalyzer.java} (76%) diff --git a/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/FileCodeAnalyzer.java b/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/FileCodeAnalyzer.java new file mode 100644 index 0000000..cf4dd0c --- /dev/null +++ b/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/FileCodeAnalyzer.java @@ -0,0 +1,34 @@ +/* + * Copyright (c) 2021, WSO2 Inc. (http://www.wso2.org) All Rights Reserved. + * + * WSO2 Inc. licenses this file to you under the Apache License, + * Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package io.ballerina.stdlib.file.compiler; + +import io.ballerina.compiler.syntax.tree.SyntaxKind; +import io.ballerina.projects.plugins.CodeAnalysisContext; +import io.ballerina.projects.plugins.CodeAnalyzer; +import io.ballerina.stdlib.file.compiler.staticcodeanalyzer.FileServiceValidator; + +/** + * File code analyser. + */ +public class FileCodeAnalyzer extends CodeAnalyzer { + @Override + public void init(CodeAnalysisContext codeAnalysisContext) { + codeAnalysisContext.addSyntaxNodeAnalysisTask(new FileServiceValidator(), SyntaxKind.SERVICE_DECLARATION); + } +} diff --git a/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/FileCompilerPlugin.java b/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/FileCompilerPlugin.java index 7bcee6c..80f4320 100644 --- a/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/FileCompilerPlugin.java +++ b/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/FileCompilerPlugin.java @@ -21,7 +21,7 @@ import io.ballerina.projects.plugins.CompilerPlugin; import io.ballerina.projects.plugins.CompilerPluginContext; import io.ballerina.scan.ScannerContext; -import io.ballerina.stdlib.file.compiler.staticcodeanalyzer.FileCodeAnalyzer; +import io.ballerina.stdlib.file.compiler.staticcodeanalyzer.FileStaticCodeAnalyzer; import static io.ballerina.stdlib.file.compiler.Constants.SCANNER_CONTEXT; @@ -32,9 +32,10 @@ public class FileCompilerPlugin extends CompilerPlugin { @Override public void init(CompilerPluginContext context) { + context.addCodeAnalyzer(new FileCodeAnalyzer()); Object object = context.userData().get(SCANNER_CONTEXT); if (object instanceof ScannerContext scannerContext) { - context.addCodeAnalyzer(new FileCodeAnalyzer(scannerContext.getReporter())); + context.addCodeAnalyzer(new FileStaticCodeAnalyzer(scannerContext.getReporter())); } } } diff --git a/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/FilePathInjectionAnalyzer.java b/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/FilePathInjectionAnalyzer.java index b5ff1de..1d01db3 100644 --- a/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/FilePathInjectionAnalyzer.java +++ b/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/FilePathInjectionAnalyzer.java @@ -98,18 +98,17 @@ private boolean isSafePath(FunctionCallExpressionNode functionCall, SyntaxNodeAn // Direct concatenation detection if (argument instanceof BinaryExpressionNode binaryExpression) { if (binaryExpression.operator().kind() == SyntaxKind.PLUS_TOKEN) { - return false; // Unsafe concatenation detected + return false; } } - // Variable reference detection if (argument instanceof SimpleNameReferenceNode variableRef) { - return isVariableSafe(variableRef, context); + return isVariableSafe(variableRef); } return true; } - private boolean isVariableSafe(SimpleNameReferenceNode variableRef, SyntaxNodeAnalysisContext context) { + private boolean isVariableSafe(SimpleNameReferenceNode variableRef) { String variableName = variableRef.name().text(); Node currentNode = variableRef.parent(); while (currentNode != null) { @@ -149,21 +148,17 @@ private boolean isFunctionParameter(SimpleNameReferenceNode variableRef) { Node currentNode = variableRef.parent(); while (currentNode != null) { if (currentNode instanceof FunctionDefinitionNode functionDef) { - // Check if function is public - boolean isPublic = functionDef.qualifierList().stream() - .anyMatch(q -> q.kind() == SyntaxKind.PUBLIC_KEYWORD); - // Iterate over function parameters to check direct reference for (ParameterNode param : functionDef.functionSignature().parameters()) { if (param instanceof RequiredParameterNode reqParam) { // Check direct parameter reference if (reqParam.paramName().isPresent() && reqParam.paramName().get().text().equals(paramName)) { - return !isPublic; + return false; } // Check indirect reference chain (assignments) if (isIndirectFunctionParameter(variableRef, reqParam)) { - return !isPublic; + return false; } } } @@ -221,7 +216,6 @@ private boolean isIndirectFunctionParameter(SimpleNameReferenceNode variableRef, */ private boolean isIndirectFunctionParameterFromBinary(BinaryExpressionNode binaryExpr, RequiredParameterNode reqParam) { - // Check both the left and right sides of the binary expression if (binaryExpr.lhsExpr() instanceof SimpleNameReferenceNode leftRef) { if (leftRef.name().text().equals(reqParam.paramName().get().text())) { return true; diff --git a/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/FileCodeAnalyzer.java b/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/FileStaticCodeAnalyzer.java similarity index 76% rename from compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/FileCodeAnalyzer.java rename to compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/FileStaticCodeAnalyzer.java index b6565d9..e4cc1c4 100644 --- a/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/FileCodeAnalyzer.java +++ b/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/FileStaticCodeAnalyzer.java @@ -1,7 +1,7 @@ /* - * Copyright (c) 2021, WSO2 Inc. (http://www.wso2.org) All Rights Reserved. + * Copyright (c) 2025, WSO2 LLC. (http://www.wso2.org) * - * WSO2 Inc. licenses this file to you under the Apache License, + * WSO2 LLC. licenses this file to you under the Apache License, * Version 2.0 (the "License"); you may not use this file except * in compliance with the License. * You may obtain a copy of the License at @@ -24,18 +24,17 @@ import io.ballerina.scan.Reporter; /** - * File code analyser. + * File static code analyser. */ -public class FileCodeAnalyzer extends CodeAnalyzer { +public class FileStaticCodeAnalyzer extends CodeAnalyzer { private final Reporter reporter; - public FileCodeAnalyzer(Reporter reporter) { + public FileStaticCodeAnalyzer(Reporter reporter) { this.reporter = reporter; } @Override public void init(CodeAnalysisContext codeAnalysisContext) { - codeAnalysisContext.addSyntaxNodeAnalysisTask(new FileServiceValidator(), SyntaxKind.SERVICE_DECLARATION); codeAnalysisContext.addSyntaxNodeAnalysisTask(new InsecureDirectoryAccessAnalyzer(reporter), SyntaxKind.FUNCTION_CALL); codeAnalysisContext.addSyntaxNodeAnalysisTask(new FilePathInjectionAnalyzer(reporter), diff --git a/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/RuleFactory.java b/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/RuleFactory.java index a7af332..cb0b713 100644 --- a/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/RuleFactory.java +++ b/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/RuleFactory.java @@ -25,6 +25,10 @@ * {@code RuleFactory} contains the logic to create a {@link Rule}. */ public class RuleFactory { + + private RuleFactory() { + } + public static Rule createRule(int id, String description, RuleKind kind) { return new RuleImpl(id, description, kind); } From 3421d2d44a96ff1f0bb1d9dfb45d833e2f869bf6 Mon Sep 17 00:00:00 2001 From: SachinAkash01 Date: Thu, 27 Feb 2025 15:14:06 +0530 Subject: [PATCH 11/17] Simplify if-else statements --- .../FilePathInjectionAnalyzer.java | 27 +++++++------------ 1 file changed, 10 insertions(+), 17 deletions(-) diff --git a/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/FilePathInjectionAnalyzer.java b/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/FilePathInjectionAnalyzer.java index 1d01db3..80f383f 100644 --- a/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/FilePathInjectionAnalyzer.java +++ b/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/FilePathInjectionAnalyzer.java @@ -69,7 +69,7 @@ public void perform(SyntaxNodeAnalysisContext context) { // Detect vulnerable file function calls if (FILE_REMOVE.equals(functionName) || FILE_READ.equals(functionName) || FILE_WRITE.equals(functionName)) { - if (!isSafePath(functionCall, context)) { + if (!isSafePath(functionCall)) { Location location = functionCall.location(); this.reporter.reportIssue(document, location, AVOID_PATH_INJECTION.getId()); } @@ -80,7 +80,7 @@ public static Document getDocument(SyntaxNodeAnalysisContext context) { return context.currentPackage().module(context.moduleId()).document(context.documentId()); } - private boolean isSafePath(FunctionCallExpressionNode functionCall, SyntaxNodeAnalysisContext context) { + private boolean isSafePath(FunctionCallExpressionNode functionCall) { NodeList arguments = functionCall.arguments(); if (arguments.isEmpty()) { return true; @@ -96,10 +96,9 @@ private boolean isSafePath(FunctionCallExpressionNode functionCall, SyntaxNodeAn } // Direct concatenation detection - if (argument instanceof BinaryExpressionNode binaryExpression) { - if (binaryExpression.operator().kind() == SyntaxKind.PLUS_TOKEN) { - return false; - } + if (argument instanceof BinaryExpressionNode binaryExpression && + binaryExpression.operator().kind() == SyntaxKind.PLUS_TOKEN) { + return false; } if (argument instanceof SimpleNameReferenceNode variableRef) { @@ -126,7 +125,6 @@ private boolean isVariableSafe(SimpleNameReferenceNode variableRef) { return isFunctionParameter(variableRef); } } - // Check if assigned directly from a function parameter if (varDecl.initializer().orElse(null) instanceof SimpleNameReferenceNode) { SimpleNameReferenceNode initializer = (SimpleNameReferenceNode) varDecl.initializer().get(); @@ -216,16 +214,11 @@ private boolean isIndirectFunctionParameter(SimpleNameReferenceNode variableRef, */ private boolean isIndirectFunctionParameterFromBinary(BinaryExpressionNode binaryExpr, RequiredParameterNode reqParam) { - if (binaryExpr.lhsExpr() instanceof SimpleNameReferenceNode leftRef) { - if (leftRef.name().text().equals(reqParam.paramName().get().text())) { - return true; - } - } - if (binaryExpr.rhsExpr() instanceof SimpleNameReferenceNode rightRef) { - if (rightRef.name().text().equals(reqParam.paramName().get().text())) { - return true; - } + if (binaryExpr.lhsExpr() instanceof SimpleNameReferenceNode leftRef && + leftRef.name().text().equals(reqParam.paramName().get().text())) { + return true; } - return false; + return binaryExpr.rhsExpr() instanceof SimpleNameReferenceNode rightRef && + rightRef.name().text().equals(reqParam.paramName().get().text()); } } From 51221559e448de994d78976e341c5bbf44cf2fc7 Mon Sep 17 00:00:00 2001 From: SachinAkash01 Date: Thu, 27 Feb 2025 15:36:25 +0530 Subject: [PATCH 12/17] Improve code quality --- .../file/compiler/FileCodeAnalyzer.java | 1 - .../FileServiceValidator.java | 3 +- .../FilePathInjectionAnalyzer.java | 146 +++++++++--------- 3 files changed, 76 insertions(+), 74 deletions(-) rename compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/{staticcodeanalyzer => }/FileServiceValidator.java (98%) diff --git a/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/FileCodeAnalyzer.java b/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/FileCodeAnalyzer.java index cf4dd0c..85bebb3 100644 --- a/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/FileCodeAnalyzer.java +++ b/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/FileCodeAnalyzer.java @@ -21,7 +21,6 @@ import io.ballerina.compiler.syntax.tree.SyntaxKind; import io.ballerina.projects.plugins.CodeAnalysisContext; import io.ballerina.projects.plugins.CodeAnalyzer; -import io.ballerina.stdlib.file.compiler.staticcodeanalyzer.FileServiceValidator; /** * File code analyser. diff --git a/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/FileServiceValidator.java b/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/FileServiceValidator.java similarity index 98% rename from compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/FileServiceValidator.java rename to compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/FileServiceValidator.java index 1b9adcd..851c3ec 100644 --- a/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/FileServiceValidator.java +++ b/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/FileServiceValidator.java @@ -16,7 +16,7 @@ * under the License. */ -package io.ballerina.stdlib.file.compiler.staticcodeanalyzer; +package io.ballerina.stdlib.file.compiler; import io.ballerina.compiler.api.symbols.ServiceDeclarationSymbol; import io.ballerina.compiler.api.symbols.Symbol; @@ -33,7 +33,6 @@ import io.ballerina.compiler.syntax.tree.SyntaxKind; import io.ballerina.projects.plugins.AnalysisTask; import io.ballerina.projects.plugins.SyntaxNodeAnalysisContext; -import io.ballerina.stdlib.file.compiler.ErrorCodes; import io.ballerina.tools.diagnostics.Diagnostic; import io.ballerina.tools.diagnostics.DiagnosticFactory; import io.ballerina.tools.diagnostics.DiagnosticInfo; diff --git a/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/FilePathInjectionAnalyzer.java b/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/FilePathInjectionAnalyzer.java index 80f383f..31edbb0 100644 --- a/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/FilePathInjectionAnalyzer.java +++ b/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/FilePathInjectionAnalyzer.java @@ -67,12 +67,10 @@ public void perform(SyntaxNodeAnalysisContext context) { String functionName = functionCall.functionName().toString(); // Detect vulnerable file function calls - if (FILE_REMOVE.equals(functionName) || FILE_READ.equals(functionName) || - FILE_WRITE.equals(functionName)) { - if (!isSafePath(functionCall)) { - Location location = functionCall.location(); - this.reporter.reportIssue(document, location, AVOID_PATH_INJECTION.getId()); - } + if ((FILE_REMOVE.equals(functionName) || FILE_READ.equals(functionName) || + FILE_WRITE.equals(functionName)) && !isSafePath(functionCall)) { + Location location = functionCall.location(); + this.reporter.reportIssue(document, location, AVOID_PATH_INJECTION.getId()); } } @@ -110,29 +108,16 @@ private boolean isSafePath(FunctionCallExpressionNode functionCall) { private boolean isVariableSafe(SimpleNameReferenceNode variableRef) { String variableName = variableRef.name().text(); Node currentNode = variableRef.parent(); + while (currentNode != null) { if (currentNode instanceof FunctionBodyBlockNode functionBody) { for (StatementNode statement : functionBody.statements()) { - if (statement instanceof VariableDeclarationNode varDecl) { - if (varDecl.typedBindingPattern().bindingPattern() instanceof - CaptureBindingPatternNode bindingPattern) { - if (bindingPattern.variableName().text().equals(variableName)) { - // Check if assigned using concatenation - if (varDecl.initializer().orElse(null) instanceof BinaryExpressionNode) { - BinaryExpressionNode binaryExpr = (BinaryExpressionNode) - varDecl.initializer().get(); - if (binaryExpr.operator().kind() == SyntaxKind.PLUS_TOKEN) { - return isFunctionParameter(variableRef); - } - } - if (varDecl.initializer().orElse(null) instanceof SimpleNameReferenceNode) { - SimpleNameReferenceNode initializer = (SimpleNameReferenceNode) - varDecl.initializer().get(); - return isFunctionParameter(initializer); - } - return true; - } + if (statement instanceof VariableDeclarationNode varDecl && + isMatchingVariable(varDecl, variableName)) { + if (hasConcatenationAssignment(varDecl) || isAssignedFromFunctionParameter(varDecl)) { + return isFunctionParameter(variableRef); } + return true; } } } @@ -141,24 +126,29 @@ private boolean isVariableSafe(SimpleNameReferenceNode variableRef) { return true; } + private boolean isMatchingVariable(VariableDeclarationNode varDecl, String variableName) { + return varDecl.typedBindingPattern().bindingPattern() instanceof CaptureBindingPatternNode bindingPattern + && bindingPattern.variableName().text().equals(variableName); + } + + private boolean hasConcatenationAssignment(VariableDeclarationNode varDecl) { + return varDecl.initializer().orElse(null) instanceof BinaryExpressionNode binaryExpr + && binaryExpr.operator().kind() == SyntaxKind.PLUS_TOKEN; + } + + private boolean isAssignedFromFunctionParameter(VariableDeclarationNode varDecl) { + return varDecl.initializer().orElse(null) instanceof SimpleNameReferenceNode; + } + private boolean isFunctionParameter(SimpleNameReferenceNode variableRef) { String paramName = variableRef.name().text(); Node currentNode = variableRef.parent(); + while (currentNode != null) { if (currentNode instanceof FunctionDefinitionNode functionDef) { - // Iterate over function parameters to check direct reference - for (ParameterNode param : functionDef.functionSignature().parameters()) { - if (param instanceof RequiredParameterNode reqParam) { - // Check direct parameter reference - if (reqParam.paramName().isPresent() && - reqParam.paramName().get().text().equals(paramName)) { - return false; - } - // Check indirect reference chain (assignments) - if (isIndirectFunctionParameter(variableRef, reqParam)) { - return false; - } - } + if (hasDirectParameterReference(functionDef, paramName) || + hasIndirectParameterReference(variableRef, functionDef)) { + return false; } } currentNode = currentNode.parent(); @@ -166,40 +156,35 @@ private boolean isFunctionParameter(SimpleNameReferenceNode variableRef) { return true; } - /** - * Checks if the variable is indirectly referencing a function parameter - * by tracing back the assignment chain. - */ + private boolean hasDirectParameterReference(FunctionDefinitionNode functionDef, String paramName) { + for (ParameterNode param : functionDef.functionSignature().parameters()) { + if (param instanceof RequiredParameterNode reqParam && reqParam.paramName().isPresent() + && reqParam.paramName().get().text().equals(paramName)) { + return true; + } + } + return false; + } + + private boolean hasIndirectParameterReference(SimpleNameReferenceNode variableRef, + FunctionDefinitionNode functionDef) { + for (ParameterNode param : functionDef.functionSignature().parameters()) { + if (param instanceof RequiredParameterNode reqParam && isIndirectFunctionParameter(variableRef, reqParam)) { + return true; + } + } + return false; + } + private boolean isIndirectFunctionParameter(SimpleNameReferenceNode variableRef, RequiredParameterNode reqParam) { Node currentNode = variableRef.parent(); + while (currentNode != null) { if (currentNode instanceof FunctionBodyBlockNode functionBody) { for (StatementNode statement : functionBody.statements()) { - if (statement instanceof VariableDeclarationNode varDecl) { - if (varDecl.typedBindingPattern().bindingPattern() instanceof - CaptureBindingPatternNode bindingPattern) { - if (bindingPattern.variableName().text().equals(variableRef.name().text())) { - // Now check if this variable is assigned to another variable - if (varDecl.initializer().isPresent()) { - ExpressionNode initializer = varDecl.initializer().get(); - // If it's a reference to the function parameter, return true - if (initializer instanceof SimpleNameReferenceNode initializerRef) { - if (initializerRef.name().text().equals(reqParam.paramName().get().text())) { - return true; - } - } - // If it's a binary expression (like concatenation), recurse - if (initializer instanceof BinaryExpressionNode binaryExpr) { - if (binaryExpr.operator().kind() == SyntaxKind.PLUS_TOKEN) { - // Recursively check both sides of the binary expression - if (isIndirectFunctionParameterFromBinary(binaryExpr, reqParam)) { - return true; - } - } - } - } - } - } + if (statement instanceof VariableDeclarationNode varDecl + && isBindingPatternMatch(varDecl, variableRef)) { + return checkVariableInitializer(varDecl, reqParam); } } } @@ -208,10 +193,29 @@ private boolean isIndirectFunctionParameter(SimpleNameReferenceNode variableRef, return false; } - /** - * Recursively checks both sides of the binary expression (e.g., concatenation) for a reference to the - * function parameter. - */ + private boolean isBindingPatternMatch(VariableDeclarationNode varDecl, SimpleNameReferenceNode variableRef) { + return varDecl.typedBindingPattern().bindingPattern() instanceof CaptureBindingPatternNode bindingPattern + && bindingPattern.variableName().text().equals(variableRef.name().text()); + } + + private boolean checkVariableInitializer(VariableDeclarationNode varDecl, RequiredParameterNode reqParam) { + ExpressionNode initializer = varDecl.initializer().orElse(null); + if (initializer == null) { + return false; + } + + if (initializer instanceof SimpleNameReferenceNode initializerRef) { + return initializerRef.name().text().equals(reqParam.paramName().get().text()); + } + + if (initializer instanceof BinaryExpressionNode binaryExpr) { + return binaryExpr.operator().kind() == SyntaxKind.PLUS_TOKEN + && isIndirectFunctionParameterFromBinary(binaryExpr, reqParam); + } + + return false; + } + private boolean isIndirectFunctionParameterFromBinary(BinaryExpressionNode binaryExpr, RequiredParameterNode reqParam) { if (binaryExpr.lhsExpr() instanceof SimpleNameReferenceNode leftRef && From 012e1c469f067680326db5de39f26ea1cce1771c Mon Sep 17 00:00:00 2001 From: SachinAkash01 Date: Thu, 27 Feb 2025 15:40:12 +0530 Subject: [PATCH 13/17] Simplify if-else statements --- .../staticcodeanalyzer/FilePathInjectionAnalyzer.java | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/FilePathInjectionAnalyzer.java b/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/FilePathInjectionAnalyzer.java index 31edbb0..c1965da 100644 --- a/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/FilePathInjectionAnalyzer.java +++ b/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/FilePathInjectionAnalyzer.java @@ -145,11 +145,10 @@ private boolean isFunctionParameter(SimpleNameReferenceNode variableRef) { Node currentNode = variableRef.parent(); while (currentNode != null) { - if (currentNode instanceof FunctionDefinitionNode functionDef) { - if (hasDirectParameterReference(functionDef, paramName) || - hasIndirectParameterReference(variableRef, functionDef)) { - return false; - } + if (currentNode instanceof FunctionDefinitionNode functionDef + && (hasDirectParameterReference(functionDef, paramName) || + hasIndirectParameterReference(variableRef, functionDef))) { + return false; } currentNode = currentNode.parent(); } From 2ff5fa941c68e89da66b00702ae294a7af308fb7 Mon Sep 17 00:00:00 2001 From: SachinAkash01 Date: Fri, 28 Feb 2025 15:13:11 +0530 Subject: [PATCH 14/17] Add jacoco plugin to the compiler plugin test --- compiler-plugin-test/build.gradle | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/compiler-plugin-test/build.gradle b/compiler-plugin-test/build.gradle index 01b22c8..a2f2a37 100644 --- a/compiler-plugin-test/build.gradle +++ b/compiler-plugin-test/build.gradle @@ -20,10 +20,16 @@ plugins { id 'java' id 'checkstyle' id 'com.github.spotbugs' + id 'jacoco' } description = 'Ballerina - file Compiler Plugin Test' +jacoco { + toolVersion = "${jacocoVersion}" + reportsDirectory = file("$project.buildDir/reports/jacoco") +} + def ballerinaDist = "${project.rootDir}/target/ballerina-runtime" dependencies { From e0923a5db5901886354bc29c4e00eb017ee7ce81 Mon Sep 17 00:00:00 2001 From: SachinAkash01 Date: Fri, 28 Feb 2025 15:14:13 +0530 Subject: [PATCH 15/17] Add jacoco plugin version --- gradle.properties | 1 + 1 file changed, 1 insertion(+) diff --git a/gradle.properties b/gradle.properties index 192afed..c64cc69 100644 --- a/gradle.properties +++ b/gradle.properties @@ -20,4 +20,5 @@ stdlibOsVersion=1.9.0 observeVersion=1.4.0 observeInternalVersion=1.4.0 +jacocoVersion=0.8.10 balScanVersion=0.5.0 From ae15fb110ebe28a355c8fb326d7d4a411475b2d7 Mon Sep 17 00:00:00 2001 From: SachinAkash01 Date: Fri, 28 Feb 2025 15:25:41 +0530 Subject: [PATCH 16/17] Add jacocoTestReport --- compiler-plugin-test/build.gradle | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/compiler-plugin-test/build.gradle b/compiler-plugin-test/build.gradle index a2f2a37..c358e09 100644 --- a/compiler-plugin-test/build.gradle +++ b/compiler-plugin-test/build.gradle @@ -30,6 +30,15 @@ jacoco { reportsDirectory = file("$project.buildDir/reports/jacoco") } +jacocoTestReport { + reports { + xml.required = true + csv.required = false + html.required = false + } + sourceSets project(':file-compiler-plugin').sourceSets.main +} + def ballerinaDist = "${project.rootDir}/target/ballerina-runtime" dependencies { From f91a8d53c2409c3205a782bddb6f6252793037f2 Mon Sep 17 00:00:00 2001 From: SachinAkash01 Date: Mon, 3 Mar 2025 11:14:26 +0530 Subject: [PATCH 17/17] Check for file import aliases and improve code quality --- .../stdlib/file/compiler/Constants.java | 35 +++++++++-- .../FilePathInjectionAnalyzer.java | 59 +++++++++++-------- .../InsecureDirectoryAccessAnalyzer.java | 10 ++-- 3 files changed, 72 insertions(+), 32 deletions(-) diff --git a/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/Constants.java b/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/Constants.java index 9de8c28..65d5310 100644 --- a/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/Constants.java +++ b/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/Constants.java @@ -18,6 +18,8 @@ package io.ballerina.stdlib.file.compiler; +import java.util.List; + /** * Constants related to compiler plugin implementation. */ @@ -28,8 +30,33 @@ private Constants() {} public static final String OS = "os"; public static final String GET_ENV = "getEnv"; public static final String FILE = "file"; - public static final String FILE_CREATE = "create"; - public static final String FILE_REMOVE = "file:remove"; - public static final String FILE_READ = "file:read"; - public static final String FILE_WRITE = "file:write"; + public static final String BALLERINA_ORG = "ballerina"; + + public static final List FILE_FUNCTIONS = List.of( + "getAbsolutePath", + "isAbsolutePath", + "basename", + "parentPath", + "normalizePath", + "splitPath", + "joinPath", + "relativePath", + "joinPath", + "test", + "copy", + "readDir", + "read", + "write", + "remove", + "create", + "getMetaData", + "createTemp", + "createTempDir" + ); + + public static final List PUBLIC_DIRECTORIES = List.of( + "\"TMP\"", + "\"TEMP\"", + "\"TMPDIR\"" + ); } diff --git a/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/FilePathInjectionAnalyzer.java b/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/FilePathInjectionAnalyzer.java index c1965da..1c70feb 100644 --- a/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/FilePathInjectionAnalyzer.java +++ b/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/FilePathInjectionAnalyzer.java @@ -25,6 +25,9 @@ import io.ballerina.compiler.syntax.tree.FunctionBodyBlockNode; import io.ballerina.compiler.syntax.tree.FunctionCallExpressionNode; import io.ballerina.compiler.syntax.tree.FunctionDefinitionNode; +import io.ballerina.compiler.syntax.tree.ImportOrgNameNode; +import io.ballerina.compiler.syntax.tree.ImportPrefixNode; +import io.ballerina.compiler.syntax.tree.ModulePartNode; import io.ballerina.compiler.syntax.tree.Node; import io.ballerina.compiler.syntax.tree.NodeList; import io.ballerina.compiler.syntax.tree.ParameterNode; @@ -38,11 +41,13 @@ import io.ballerina.projects.plugins.AnalysisTask; import io.ballerina.projects.plugins.SyntaxNodeAnalysisContext; import io.ballerina.scan.Reporter; -import io.ballerina.tools.diagnostics.Location; -import static io.ballerina.stdlib.file.compiler.Constants.FILE_REMOVE; -import static io.ballerina.stdlib.file.compiler.Constants.FILE_READ; -import static io.ballerina.stdlib.file.compiler.Constants.FILE_WRITE; +import java.util.ArrayList; +import java.util.List; + +import static io.ballerina.stdlib.file.compiler.Constants.BALLERINA_ORG; +import static io.ballerina.stdlib.file.compiler.Constants.FILE; +import static io.ballerina.stdlib.file.compiler.Constants.FILE_FUNCTIONS; import static io.ballerina.stdlib.file.compiler.staticcodeanalyzer.FileRule.AVOID_PATH_INJECTION; /** @@ -63,14 +68,29 @@ public void perform(SyntaxNodeAnalysisContext context) { } Document document = getDocument(context); + List importPrefix = new ArrayList<>(); + if (document.syntaxTree().rootNode() instanceof ModulePartNode modulePartNode) { + importPrefix = modulePartNode.imports().stream() + .filter(importDeclarationNode -> { + ImportOrgNameNode importOrgNameNode = importDeclarationNode.orgName().orElse(null); + return importOrgNameNode != null && BALLERINA_ORG.equals(importOrgNameNode.orgName().text()); + }) + .filter(importDeclarationNode -> importDeclarationNode.moduleName().stream().anyMatch( + moduleNameNode -> FILE.equals(moduleNameNode.text()))) + .map(importDeclarationNode -> { + ImportPrefixNode importPrefixNode = importDeclarationNode.prefix().orElse(null); + return importPrefixNode != null ? importPrefixNode.prefix().text() : FILE; + }).toList(); + } String functionName = functionCall.functionName().toString(); - // Detect vulnerable file function calls - if ((FILE_REMOVE.equals(functionName) || FILE_READ.equals(functionName) || - FILE_WRITE.equals(functionName)) && !isSafePath(functionCall)) { - Location location = functionCall.location(); - this.reporter.reportIssue(document, location, AVOID_PATH_INJECTION.getId()); + boolean isFileOperation = importPrefix.stream().anyMatch(prefix -> + FILE_FUNCTIONS.stream().anyMatch(func -> functionName.equals(prefix + ":" + func)) + ); + + if (isFileOperation && !isSafePath(functionCall)) { + this.reporter.reportIssue(document, functionCall.location(), AVOID_PATH_INJECTION.getId()); } } @@ -93,7 +113,6 @@ private boolean isSafePath(FunctionCallExpressionNode functionCall) { return true; } - // Direct concatenation detection if (argument instanceof BinaryExpressionNode binaryExpression && binaryExpression.operator().kind() == SyntaxKind.PLUS_TOKEN) { return false; @@ -199,20 +218,14 @@ private boolean isBindingPatternMatch(VariableDeclarationNode varDecl, SimpleNam private boolean checkVariableInitializer(VariableDeclarationNode varDecl, RequiredParameterNode reqParam) { ExpressionNode initializer = varDecl.initializer().orElse(null); - if (initializer == null) { - return false; - } - - if (initializer instanceof SimpleNameReferenceNode initializerRef) { - return initializerRef.name().text().equals(reqParam.paramName().get().text()); - } - - if (initializer instanceof BinaryExpressionNode binaryExpr) { - return binaryExpr.operator().kind() == SyntaxKind.PLUS_TOKEN + return switch (initializer) { + case null -> false; + case SimpleNameReferenceNode initializerRef -> + initializerRef.name().text().equals(reqParam.paramName().get().text()); + case BinaryExpressionNode binaryExpr -> binaryExpr.operator().kind() == SyntaxKind.PLUS_TOKEN && isIndirectFunctionParameterFromBinary(binaryExpr, reqParam); - } - - return false; + default -> false; + }; } private boolean isIndirectFunctionParameterFromBinary(BinaryExpressionNode binaryExpr, diff --git a/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/InsecureDirectoryAccessAnalyzer.java b/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/InsecureDirectoryAccessAnalyzer.java index 9447e0a..0fbde34 100644 --- a/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/InsecureDirectoryAccessAnalyzer.java +++ b/compiler-plugin/src/main/java/io/ballerina/stdlib/file/compiler/staticcodeanalyzer/InsecureDirectoryAccessAnalyzer.java @@ -33,10 +33,11 @@ import io.ballerina.tools.diagnostics.Location; import org.ballerinalang.model.tree.expressions.StringTemplateLiteralNode; +import static io.ballerina.stdlib.file.compiler.Constants.FILE_FUNCTIONS; import static io.ballerina.stdlib.file.compiler.Constants.OS; import static io.ballerina.stdlib.file.compiler.Constants.GET_ENV; import static io.ballerina.stdlib.file.compiler.Constants.FILE; -import static io.ballerina.stdlib.file.compiler.Constants.FILE_CREATE; +import static io.ballerina.stdlib.file.compiler.Constants.PUBLIC_DIRECTORIES; import static io.ballerina.stdlib.file.compiler.staticcodeanalyzer.FileRule.AVOID_INSECURE_DIRECTORY_ACCESS; public class InsecureDirectoryAccessAnalyzer implements AnalysisTask { @@ -74,8 +75,7 @@ private boolean isFileMethodCall(FunctionCallExpressionNode functionCall) { String functionName = qNode.identifier().text(); if (modulePrefix.equals(OS) && functionName.equals(GET_ENV)) { String envVarName = currentMethodCall.arguments().get(0).toString(); - if (envVarName.equals("\"TMP\"") || envVarName.equals("\"TEMP\"") || - envVarName.equals("\"TMPDIR\"")) { + if (PUBLIC_DIRECTORIES.contains(envVarName)) { return true; } } @@ -84,7 +84,7 @@ private boolean isFileMethodCall(FunctionCallExpressionNode functionCall) { if (currentMethodCall.functionName() instanceof QualifiedNameReferenceNode fileCallNode) { String fileModulePrefix = fileCallNode.modulePrefix().text(); String fileFunctionName = fileCallNode.identifier().text(); - if (fileModulePrefix.equals(FILE) && fileFunctionName.equals(FILE_CREATE)) { + if (fileModulePrefix.equals(FILE) && FILE_FUNCTIONS.contains(fileFunctionName)) { SeparatedNodeList arguments = currentMethodCall.arguments(); if (arguments != null && !arguments.isEmpty()) { FunctionArgumentNode pathArg = arguments.get(0); @@ -112,6 +112,6 @@ private boolean isFileMethodCall(FunctionCallExpressionNode functionCall) { } private boolean isInsecureDirectory(String filePath) { - return filePath.contains("/tmp") || filePath.contains("TMP") || filePath.contains("TEMP"); + return PUBLIC_DIRECTORIES.contains(filePath); } }