add sarif parsing basics

Author: nahsra

core-codemods/src/main/java/io/codemodder/codemods/SQLParameterizer.java

@@ -29,8 +29,11 @@
 import java.util.stream.Collectors;
 import org.javatuples.Pair;
 
-/** Contains most of the logic for the SQLParameterizerCodemod. */
-final class SQLParameterizer {
+/**
+ * Contains most of the logic for detecting and fixing parameterizable SQL statements for a given
+ * {@link MethodCallExpr}.
+ */
+public final class SQLParameterizer {
 
   private static final String preparedStatementNamePrefix = "stmt";
   private static final String preparedStatementNamePrefixAlternative = "statement";
@@ -39,7 +42,7 @@ final class SQLParameterizer {
 
   private CompilationUnit compilationUnit;
 
-  SQLParameterizer(final MethodCallExpr methodCallExpr) {
+  public SQLParameterizer(final MethodCallExpr methodCallExpr) {
     this.executeCall = Objects.requireNonNull(methodCallExpr);
     this.compilationUnit = null;
   }
@@ -500,7 +503,7 @@ private void fix(
    * Checks if {@code methodCall} is a query call that needs to be fixed and fixes if that's the
    * case.
    */
-  boolean checkAndFix() {
+  public boolean checkAndFix() {
 
     if (executeCall.findCompilationUnit().isPresent()) {
       this.compilationUnit = executeCall.findCompilationUnit().get();

framework/codemodder-base/src/main/java/io/codemodder/LazyLoadingRuleSarif.java

@@ -38,9 +38,9 @@ private void checkInitialized() {
   }
 
   @Override
-  public List<Result> getResultsByPath(final Path path) {
+  public List<Result> getResultsByLocationPath(final Path path) {
     checkInitialized();
-    return ruleSarif.getResultsByPath(path);
+    return ruleSarif.getResultsByLocationPath(path);
   }
 
   @Override

framework/codemodder-base/src/main/java/io/codemodder/RuleSarif.java

@@ -21,12 +21,12 @@ public interface RuleSarif {
   List<Region> getRegionsFromResultsByRule(Path path);
 
   /**
-   * Get all the SARIF results with the matching path
+   * Get all the SARIF results with the matching path for the first location field
    *
    * @param path the file being scanned
    * @return the results associated with the given file
    */
-  List<Result> getResultsByPath(Path path);
+  List<Result> getResultsByLocationPath(Path path);
 
   /** Return the entire SARIF as a model in case more comprehensive inspection is needed. */
   SarifSchema210 rawDocument();
@@ -46,7 +46,7 @@ public List<Region> getRegionsFromResultsByRule(final Path path) {
     }
 
     @Override
-    public List<Result> getResultsByPath(final Path path) {
+    public List<Result> getResultsByLocationPath(final Path path) {
       return List.of();
     }
 

framework/codemodder-base/src/main/java/io/codemodder/SarifPluginJavaParserChanger.java

@@ -91,7 +91,7 @@ protected SarifPluginJavaParserChanger(
 
   public List<CodemodChange> visit(
       final CodemodInvocationContext context, final CompilationUnit cu) {
-    List<Result> results = sarif.getResultsByPath(context.path());
+    List<Result> results = sarif.getResultsByLocationPath(context.path());
 
     // small shortcut to avoid always executing the expensive findAll
     if (results.isEmpty()) {

framework/codemodder-base/src/main/java/io/codemodder/SarifPluginRawFileChanger.java

@@ -20,7 +20,7 @@ protected SarifPluginRawFileChanger(
 
   @Override
   public List<CodemodChange> visitFile(final CodemodInvocationContext context) {
-    List<Result> results = sarif.getResultsByPath(context.path());
+    List<Result> results = sarif.getResultsByLocationPath(context.path());
     if (!results.isEmpty()) {
       return onFileFound(context, results);
     }

gradle/build-plugins/src/main/kotlin/io.codemodder.maven-publish.gradle.kts

@@ -22,6 +22,7 @@ extensions.getByType<nebula.plugin.contacts.ContactsExtension>().run {
 
 val publicationName = "nebula"
 signing {
+    setRequired(false)
     if (providers.environmentVariable("CI").isPresent) {
         val signingKey: String? by project
         val signingPassword: String? by project

plugins/codemodder-plugin-appscan/src/main/java/io/codemodder/providers/sarif/appscan/AppScanModule.java

@@ -11,13 +11,13 @@
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
-/** Responsible for distributing the SARIFS to AppSCan based codemods based on rules. */
+/** Responsible for distributing the SARIFS to AppScan based codemods based on rules. */
 public final class AppScanModule extends AbstractModule {
 
   private final List<Class<? extends CodeChanger>> codemodTypes;
   private final List<RuleSarif> allAppScanRuleSarifs;
 
-  AppScanModule(
+  public AppScanModule(
       final List<Class<? extends CodeChanger>> codemodTypes, final List<RuleSarif> sarifs) {
     this.codemodTypes = Objects.requireNonNull(codemodTypes);
     this.allAppScanRuleSarifs = sarifs;

plugins/codemodder-plugin-appscan/src/main/java/io/codemodder/providers/sarif/appscan/AppScanRuleSarif.java

@@ -1,14 +1,16 @@
 package io.codemodder.providers.sarif.appscan;
 
-import com.contrastsecurity.sarif.Region;
-import com.contrastsecurity.sarif.Result;
-import com.contrastsecurity.sarif.SarifSchema210;
+import com.contrastsecurity.sarif.*;
 import io.codemodder.RuleSarif;
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.nio.file.FileVisitResult;
+import java.nio.file.Files;
 import java.nio.file.Path;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-import java.util.Objects;
+import java.nio.file.SimpleFileVisitor;
+import java.nio.file.attribute.BasicFileAttributes;
+import java.util.*;
+import java.util.concurrent.atomic.AtomicReference;
 
 /** A {@link RuleSarif} for AppScan results. */
 final class AppScanRuleSarif implements RuleSarif {
@@ -17,23 +19,104 @@ final class AppScanRuleSarif implements RuleSarif {
   private final String ruleId;
   private final Map<Path, List<Result>> resultsCache;
   private final Path repositoryRoot;
+  private final List<String> locations;
 
+  /** A map of a HCL SARIF "location" URIs mapped to their respective file paths. */
+  private final Map<Path, Set<Integer>> artifactLocationIndices;
+
+  /**
+   * Creates an {@link AppScanRuleSarif} that has already done the work of mapping HCL SARIF
+   * locations, which are strange combinations of class name and file path, into predictable paths.
+   */
   public AppScanRuleSarif(
       final String ruleId, final SarifSchema210 sarif, final Path repositoryRoot) {
     this.sarif = Objects.requireNonNull(sarif);
     this.ruleId = Objects.requireNonNull(ruleId);
     this.repositoryRoot = repositoryRoot;
     this.resultsCache = new HashMap<>();
+    this.locations =
+        sarif.getRuns().get(0).getArtifacts().stream()
+            .map(Artifact::getLocation)
+            .map(ArtifactLocation::getUri)
+            .map(u -> u.substring(8))
+            .toList();
+    Map<Path, Set<Integer>> artifactLocationIndices = new HashMap<>();
+
+    for (int i = 0; i < locations.size(); i++) {
+      final Integer index = i;
+      String path = locations.get(i);
+      path = path.replace('\\', '/');
+      // we have a real but partial path, now we have to find it in the repository
+      Optional<Path> existingRealPath;
+      try {
+        existingRealPath = findFileWithTrailingPath(path);
+      } catch (IOException e) {
+        throw new UncheckedIOException(e);
+      }
+
+      // add to the map if we found a matching file
+      existingRealPath.ifPresent(
+          p ->
+              artifactLocationIndices
+                  .computeIfAbsent(repositoryRoot.relativize(p), k -> new HashSet<>())
+                  .add(index));
+    }
+    this.artifactLocationIndices = Map.copyOf(artifactLocationIndices);
+  }
+
+  private Optional<Path> findFileWithTrailingPath(final String path) throws IOException {
+    // find the files with the trailing path
+    AtomicReference<Path> found = new AtomicReference<>();
+    Files.walkFileTree(
+        repositoryRoot,
+        new SimpleFileVisitor<>() {
+          @Override
+          public FileVisitResult visitFile(final Path file, final BasicFileAttributes attrs) {
+            if (file.toString().endsWith(path)) {
+              found.set(file);
+              return FileVisitResult.TERMINATE;
+            }
+            return FileVisitResult.CONTINUE;
+          }
+        });
+    return Optional.ofNullable(found.get());
   }
 
   @Override
   public List<Region> getRegionsFromResultsByRule(final Path path) {
-    return List.of();
+    List<Result> resultsByLocationPath = getResultsByLocationPath(path);
+    return resultsByLocationPath.stream()
+        .map(result -> result.getLocations().get(0).getPhysicalLocation().getRegion())
+        .toList();
   }
 
+  /**
+   * This call receives an actual source file path, whereas the HCL results store a reference to a
+   * fully qualified class name plus ".java", e.g.:
+   *
+   * <pre>file:///org/owasp/webgoat/lessons/challenges/challenge5/Assignment5.java</pre>
+   */
   @Override
-  public List<Result> getResultsByPath(final Path path) {
-    return List.of();
+  public List<Result> getResultsByLocationPath(final Path path) {
+    return resultsCache.computeIfAbsent(
+        path,
+        p ->
+            sarif.getRuns().stream()
+                .flatMap(run -> run.getResults().stream())
+                .filter(result -> result.getRuleId().equals(ruleId))
+                .filter(
+                    result ->
+                        artifactLocationIndices.get(path) != null
+                            && artifactLocationIndices
+                                .get(path)
+                                .contains(
+                                    result
+                                        .getLocations()
+                                        .get(0)
+                                        .getPhysicalLocation()
+                                        .getArtifactLocation()
+                                        .getIndex()))
+                .toList());
   }
 
   @Override

plugins/codemodder-plugin-appscan/src/test/java/io/codemodder/providers/sarif/appscan/AppScanModuleTest.java

@@ -0,0 +1,85 @@
+package io.codemodder.providers.sarif.appscan;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.notNullValue;
+
+import com.contrastsecurity.sarif.Region;
+import com.google.inject.Guice;
+import com.google.inject.Injector;
+import io.codemodder.*;
+import io.codemodder.codetf.CodeTFReference;
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.StandardOpenOption;
+import java.util.List;
+import javax.inject.Inject;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+
+final class AppScanModuleTest {
+
+  private Path repoDir;
+
+  @Codemod(
+      id = "appscan-test:java/finds-stuff",
+      importance = Importance.LOW,
+      reviewGuidance = ReviewGuidance.MERGE_AFTER_CURSORY_REVIEW)
+  static class AppScanSarifTestCodemod implements CodeChanger {
+    private final RuleSarif ruleSarif;
+
+    @Inject
+    AppScanSarifTestCodemod(@ProvidedAppScanScan(ruleId = "SA2813462719") RuleSarif ruleSarif) {
+      this.ruleSarif = ruleSarif;
+    }
+
+    @Override
+    public String getSummary() {
+      return null;
+    }
+
+    @Override
+    public String getDescription() {
+      return null;
+    }
+
+    @Override
+    public List<CodeTFReference> getReferences() {
+      return null;
+    }
+
+    @Override
+    public String getIndividualChangeDescription(Path filePath, CodemodChange change) {
+      return null;
+    }
+  }
+
+  @BeforeEach
+  void setup(@TempDir final Path tmpDir) {
+    AppScanRuleSarifFactory factory = new AppScanRuleSarifFactory();
+    factory.build("appscan", "SA2813462719", null, null);
+    this.repoDir = tmpDir;
+  }
+
+  @Test
+  void it_works_with_appscan_sarif() throws IOException {
+    String javaCode = "class Foo { \n Object a = new Thing(); \n }";
+
+    Path javaFile = Files.createTempFile(repoDir, "HasThing", ".java");
+    Files.writeString(javaFile, javaCode, StandardOpenOption.TRUNCATE_EXISTING);
+    AppScanModule module = createModule(List.of(AppScanSarifTestCodemod.class));
+    Injector injector = Guice.createInjector(module);
+    AppScanSarifTestCodemod instance = injector.getInstance(AppScanSarifTestCodemod.class);
+
+    RuleSarif ruleSarif = instance.ruleSarif;
+    assertThat(ruleSarif, is(notNullValue()));
+    List<Region> regions = ruleSarif.getRegionsFromResultsByRule(javaFile);
+    assertThat(regions.size(), is(1));
+  }
+
+  private AppScanModule createModule(final List<Class<? extends CodeChanger>> codemodTypes) {
+    return new AppScanModule(codemodTypes, List.of());
+  }
+}

plugins/codemodder-plugin-appscan/src/test/java/io/codemodder/providers/sarif/appscan/AppScanRuleSarifFactoryTest.java

@@ -0,0 +1,68 @@
+package io.codemodder.providers.sarif.appscan;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import com.contrastsecurity.sarif.Region;
+import com.contrastsecurity.sarif.Result;
+import com.contrastsecurity.sarif.SarifSchema210;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import io.codemodder.RuleSarif;
+import java.io.File;
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.List;
+import java.util.Optional;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+
+final class AppScanRuleSarifFactoryTest {
+
+  /**
+   * In this test we'll attempt to load the SARIF file and build a {@link RuleSarif} from it, adn
+   * confirm we can get the right locations for result guid 9833fe60-ded1-ee11-9f02-14cb65725114.
+   */
+  @Test
+  void it_parses_sarif_and_maps_java_locations(@TempDir final Path tmpDir) throws IOException {
+
+    // create the webgoat file in the repository dir
+    String expectedPath =
+        "src/main/java/org/owasp/webgoat/lessons/challenges/challenge5/Assignment5.java";
+    Path actualAssignmentedJavaFilePath = tmpDir.resolve(expectedPath);
+    actualAssignmentedJavaFilePath.toFile().getParentFile().mkdirs();
+
+    // create the file contents that this SARIF has results for
+    Files.copy(Path.of("src/test/resources/Assignment5.java.txt"), actualAssignmentedJavaFilePath);
+
+    // read the SARIF file and build a RuleSarif from it
+    AppScanRuleSarifFactory appScanRuleSarifFactory = new AppScanRuleSarifFactory();
+    SarifSchema210 rawSarif =
+        new ObjectMapper()
+            .readValue(
+                new File("src/test/resources/webgoat_2023_8_binary.sarif"), SarifSchema210.class);
+    Optional<RuleSarif> sarifRef =
+        appScanRuleSarifFactory.build(
+            "HCL AppScan Static Analyzer", "SA2813462719", rawSarif, tmpDir);
+    assertThat(sarifRef.isPresent()).isTrue();
+    RuleSarif ruleSarif = sarifRef.get();
+
+    // verify the rule sarif has the right stuff
+    assertThat(ruleSarif.getRule()).isEqualTo("SA2813462719");
+    assertThat(ruleSarif.getDriver()).isEqualTo("HCL AppScan Static Analyzer");
+    assertThat(ruleSarif.rawDocument()).isEqualTo(rawSarif);
+
+    // get the results for the file path (not the weird HCL thing) and confirm we have the right
+    // results
+    Path p = Path.of(expectedPath);
+    List<Result> resultsForPath = ruleSarif.getResultsByLocationPath(p);
+    assertThat(resultsForPath).isNotEmpty();
+
+    // get the regions affected by the given file
+    List<Region> regions = ruleSarif.getRegionsFromResultsByRule(p);
+
+    // there is an injection in two form parameters in the SQL, so these 2 share the same sink
+    assertThat(regions).hasSize(2);
+    assertThat(regions.get(0).getStartLine()).isEqualTo(59);
+    assertThat(regions.get(1).getStartLine()).isEqualTo(59);
+  }
+}