Create and use shared SQL injection reporting metadata (#408)

Author: nahsra

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

@@ -7,6 +7,7 @@
 import io.codemodder.providers.sonar.ProvidedSonarScan;
 import io.codemodder.providers.sonar.RuleHotspot;
 import io.codemodder.providers.sonar.SonarRemediatingJavaParserChanger;
+import io.codemodder.remediation.GenericRemediationMetadata;
 import io.codemodder.sonar.model.Hotspot;
 import io.codemodder.sonar.model.SonarFinding;
 import java.util.List;
@@ -26,7 +27,7 @@ public final class SonarSQLInjectionCodemod extends SonarRemediatingJavaParserCh
   @Inject
   public SonarSQLInjectionCodemod(
       @ProvidedSonarScan(ruleId = "java:S2077") final RuleHotspot hotspots) {
-    super(CodemodReporterStrategy.fromClasspath(SQLParameterizerCodemod.class), hotspots);
+    super(GenericRemediationMetadata.SQL_INJECTION.reporter(), hotspots);
     this.hotspots = Objects.requireNonNull(hotspots);
     this.remediationStrategy = JavaParserSQLInjectionRemediatorStrategy.DEFAULT;
   }

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

@@ -6,7 +6,7 @@
 import io.codemodder.providers.sonar.ProvidedSonarScan;
 import io.codemodder.providers.sonar.RuleIssue;
 import io.codemodder.providers.sonar.SonarRemediatingJavaParserChanger;
-import io.codemodder.remediation.GenericVulnerabilityReporterStrategies;
+import io.codemodder.remediation.GenericRemediationMetadata;
 import io.codemodder.remediation.reflectioninjection.ReflectionInjectionRemediator;
 import io.codemodder.sonar.model.Issue;
 import java.util.Objects;
@@ -27,7 +27,7 @@ public final class SonarUnsafeReflectionRemediationCodemod
   @Inject
   public SonarUnsafeReflectionRemediationCodemod(
       @ProvidedSonarScan(ruleId = "java:S2658") final RuleIssue issues) {
-    super(GenericVulnerabilityReporterStrategies.reflectionInjectionStrategy, issues);
+    super(GenericRemediationMetadata.REFLECTION_INJECTION.reporter(), issues);
     this.remediator = ReflectionInjectionRemediator.DEFAULT;
     this.issues = Objects.requireNonNull(issues);
   }

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

@@ -6,7 +6,7 @@
 import io.codemodder.providers.sonar.ProvidedSonarScan;
 import io.codemodder.providers.sonar.RuleIssue;
 import io.codemodder.providers.sonar.SonarRemediatingJavaParserChanger;
-import io.codemodder.remediation.GenericVulnerabilityReporterStrategies;
+import io.codemodder.remediation.GenericRemediationMetadata;
 import io.codemodder.remediation.xxe.XXERemediator;
 import io.codemodder.sonar.model.Issue;
 import io.codemodder.sonar.model.SonarFinding;
@@ -26,7 +26,7 @@ public final class SonarXXECodemod extends SonarRemediatingJavaParserChanger {
 
   @Inject
   public SonarXXECodemod(@ProvidedSonarScan(ruleId = "java:S2755") final RuleIssue issues) {
-    super(GenericVulnerabilityReporterStrategies.xxeReporterStrategy, issues);
+    super(GenericRemediationMetadata.XXE.reporter(), issues);
     this.issues = Objects.requireNonNull(issues);
     this.remediationStrategy = XXERemediator.DEFAULT;
   }

framework/codemodder-base/src/main/java/io/codemodder/remediation/GenericRemediationMetadata.java

@@ -0,0 +1,26 @@
+package io.codemodder.remediation;
+
+import io.codemodder.CodeChanger;
+import io.codemodder.CodemodReporterStrategy;
+
+/** Reporter strategies that are useful without mentioning specific APIs. */
+public enum GenericRemediationMetadata {
+  XXE("xxe"),
+  JNDI("jndi-injection"),
+  HEADER_INJECTION("header-injection"),
+  REFLECTION_INJECTION("reflection-injection"),
+  SQL_INJECTION("sql-injection");
+
+  private final CodemodReporterStrategy reporter;
+
+  GenericRemediationMetadata(final String dir) {
+    this.reporter =
+        CodemodReporterStrategy.fromClasspathDirectory(
+            CodeChanger.class, "/generic-remediation-reports/" + dir);
+  }
+
+  /** Get the reporter strategy for this vulnerability class. */
+  public CodemodReporterStrategy reporter() {
+    return reporter;
+  }
+}

framework/codemodder-base/src/main/java/io/codemodder/remediation/GenericVulnerabilityReporterStrategies.java

@@ -0,0 +1,13 @@
+This change refactors SQL statements to be parameterized, rather than built by hand.
+
+Without parameterization, developers must remember to escape inputs using the rules for that database. It's usually buggy, at the least -- and sometimes vulnerable.
+
+Our changes look something like this:
+
+```diff
+- Statement stmt = connection.createStatement();
+- ResultSet rs = stmt.executeQuery("SELECT * FROM users WHERE name = '" + user + "'");
++ PreparedStatement stmt = connection.prepareStatement("SELECT * FROM users WHERE name = ?");
++ stmt.setString(1, user);
++ ResultSet rs = stmt.executeQuery();
+```

framework/codemodder-base/src/main/resources/generic-remediation-reports/sql-injection/description.md

@@ -0,0 +1,9 @@
+{
+  "summary" : "Refactored to use parameterized SQL APIs",
+  "change": "Parameterized SQL usage to prevent any bugs or vulnerabilities",
+  "reviewGuidanceIJustification" : "Although there should be no functional differences, the rewrite here is complex and should be verified by a human.",
+  "references" : [
+    "https://cheatsheetseries.owasp.org/cheatsheets/SQL_Injection_Prevention_Cheat_Sheet.html",
+    "https://cwe.mitre.org/data/definitions/89.html"
+  ]
+}

framework/codemodder-base/src/main/resources/generic-remediation-reports/sql-injection/report.json

@@ -0,0 +1,22 @@
+package io.codemodder.remediation;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import io.codemodder.CodemodChange;
+import io.codemodder.CodemodReporterStrategy;
+import java.nio.file.Path;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.EnumSource;
+
+final class GenericRemediationMetadataTest {
+
+  @ParameterizedTest
+  @EnumSource(GenericRemediationMetadata.class)
+  void it_can_find_generic_report_elements(final GenericRemediationMetadata metadata) {
+    CodemodReporterStrategy reporter = metadata.reporter();
+    assertThat(reporter.getReferences()).isNotEmpty();
+    assertThat(reporter.getDescription()).isNotEmpty();
+    assertThat(reporter.getSummary()).isNotEmpty();
+    assertThat(reporter.getChange(Path.of("Foo.java"), CodemodChange.from(5))).isNotEmpty();
+  }
+}