Add several CodeQL mappings (#457)

Adds several mappings for CodeQL rules, as well as adds a new
integration test for a more recent version of CodeQL+WebGoat 2023.8.

Author: nahsra

core-codemods/src/intTest/java/io/codemodder/integration/GitRepositoryTest.java

@@ -1,10 +1,19 @@
 package io.codemodder.integration;
 
+import static org.hamcrest.CoreMatchers.equalTo;
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+import io.codemodder.codetf.CodeTFChangesetEntry;
+import io.codemodder.codetf.CodeTFReport;
+import io.codemodder.codetf.CodeTFResult;
 import java.io.File;
 import java.io.IOException;
 import java.nio.file.Files;
 import java.nio.file.Path;
+import java.util.Collection;
 import java.util.Comparator;
+import java.util.List;
 import java.util.Objects;
 import java.util.stream.Stream;
 import org.eclipse.jgit.api.Git;
@@ -81,4 +90,61 @@ void createOutputFile() throws IOException {
     this.outputFile = Files.createTempFile("report", ".log").toFile();
     outputFile.deleteOnExit();
   }
+
+  protected void verifyNoFailedFiles(final CodeTFReport report) {
+    List<String> failedFiles =
+        report.getResults().stream()
+            .map(CodeTFResult::getFailedFiles)
+            .flatMap(Collection::stream)
+            .toList();
+    assertThat(failedFiles.size(), is(0));
+  }
+
+  protected void verifyStandardCodemodResults(final List<CodeTFChangesetEntry> fileChanges) {
+    // we only inject into a couple files
+    assertThat(
+        fileChanges.stream()
+            .map(CodeTFChangesetEntry::getPath)
+            .anyMatch(path -> path.endsWith("SerializationHelper.java")),
+        is(true));
+
+    assertThat(
+        fileChanges.stream()
+            .map(CodeTFChangesetEntry::getPath)
+            .anyMatch(path -> path.endsWith("InsecureDeserializationTask.java")),
+        is(true));
+  }
+
+  protected void verifyCodemodsHitWithChangesetCount(
+      final CodeTFReport report, final String codemodId, final int changes) {
+    List<CodeTFResult> results =
+        report.getResults().stream()
+            .filter(result -> codemodId.equals(result.getCodemod()))
+            .toList();
+    assertThat(results.size(), equalTo(1)); // should only have 1 entry per codemod
+    assertThat(results.get(0).getChangeset().size(), equalTo(changes));
+  }
+
+  protected static final String testPathIncludes =
+      "**.java,"
+          + "**/*.java,"
+          + "pom.xml,"
+          + "**/pom.xml,"
+          + "**.jsp,"
+          + "**/*.jsp,"
+          + "web.xml,"
+          + "**/web.xml,"
+          + ".github/workflows/*.yml,"
+          + ".github/workflows/*.yaml";
+
+  protected static final String testPathExcludes =
+      "**/test/**,"
+          + "**/testFixtures/**,"
+          + "**/*Test.java,"
+          + "**/intTest/**,"
+          + "**/tests/**,"
+          + "**/target/**,"
+          + "**/build/**,"
+          + "**/.mvn/**,"
+          + ".mvn/**";
 }

core-codemods/src/intTest/java/io/codemodder/integration/WebGoat20238Test.java

@@ -0,0 +1,77 @@
+package io.codemodder.integration;
+
+import static org.hamcrest.CoreMatchers.*;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+import com.fasterxml.jackson.databind.ObjectMapper;
+import io.codemodder.codemods.DefaultCodemods;
+import io.codemodder.codetf.CodeTFChangesetEntry;
+import io.codemodder.codetf.CodeTFReport;
+import io.codemodder.codetf.CodeTFResult;
+import java.io.FileReader;
+import java.util.Collection;
+import java.util.List;
+import org.junit.jupiter.api.Test;
+
+final class WebGoat20238Test extends GitRepositoryTest {
+
+  WebGoat20238Test() {
+    super("https://github.com/WebGoat/WebGoat", "main", "f9b810c5ee2d6731eb5e37172af20d276a7dfb98");
+  }
+
+  @Test
+  void it_remediates_webgoat_2023_8() throws Exception {
+
+    DefaultCodemods.main(
+        new String[] {
+          "--output",
+          outputFile.getPath(),
+          "--sarif",
+          "src/test/resources/webgoat_v2023.8_from_ghas_06_2024.sarif",
+          "--dont-exit",
+          "--path-include=" + testPathIncludes,
+          "--path-exclude=" + testPathExcludes,
+          repoDir.getPath()
+        });
+
+    ObjectMapper objectMapper = new ObjectMapper();
+
+    var report = objectMapper.readValue(new FileReader(outputFile), CodeTFReport.class);
+
+    verifyNoFailedFiles(report);
+
+    List<CodeTFChangesetEntry> fileChanges =
+        report.getResults().stream()
+            .map(CodeTFResult::getChangeset)
+            .flatMap(Collection::stream)
+            .toList();
+
+    assertThat(fileChanges.size(), is(50));
+
+    verifyStandardCodemodResults(fileChanges);
+
+    // count the changes associated with missing-jwt-signature-check from codeql
+    List<CodeTFResult> jwtResults =
+        report.getResults().stream()
+            .filter(result -> "codeql:java/missing-jwt-signature-check".equals(result.getCodemod()))
+            .toList();
+    assertThat(jwtResults.size(), equalTo(1));
+
+    // this file is also only changed by including the codeql results
+    CodeTFChangesetEntry jwtChange =
+        fileChanges.stream()
+            .filter(change -> change.getPath().endsWith("JWTRefreshEndpoint.java"))
+            .findFirst()
+            .orElseThrow();
+
+    assertThat(jwtChange.getChanges().size(), equalTo(2));
+    assertThat(jwtChange.getChanges().get(0).getLineNumber(), equalTo(113));
+    assertThat(jwtChange.getChanges().get(1).getLineNumber(), equalTo(140));
+
+    verifyCodemodsHitWithChangesetCount(report, "codeql:java/insecure-randomness", 0);
+    verifyCodemodsHitWithChangesetCount(report, "codeql:java/ssrf", 1);
+    verifyCodemodsHitWithChangesetCount(report, "codeql:java/xxe", 1);
+    verifyCodemodsHitWithChangesetCount(report, "codeql:java/sql-injection", 6);
+    verifyCodemodsHitWithChangesetCount(report, "codeql:java/insecure-cookie", 2);
+  }
+}

core-codemods/src/intTest/java/io/codemodder/integration/WebGoat822Test.java

@@ -16,7 +16,6 @@
 import java.nio.file.Path;
 import java.util.Collection;
 import java.util.List;
-import java.util.stream.Collectors;
 import org.junit.jupiter.api.Test;
 import org.mozilla.universalchardet.UniversalDetector;
 
@@ -26,29 +25,6 @@ final class WebGoat822Test extends GitRepositoryTest {
     super("https://github.com/WebGoat/WebGoat", "main", "e75cfbeb110e3d3a2ca3c8fee2754992d89c419d");
   }
 
-  private static final String testPathIncludes =
-      "**.java,"
-          + "**/*.java,"
-          + "pom.xml,"
-          + "**/pom.xml,"
-          + "**.jsp,"
-          + "**/*.jsp,"
-          + "web.xml,"
-          + "**/web.xml,"
-          + ".github/workflows/*.yml,"
-          + ".github/workflows/*.yaml";
-
-  private static final String testPathExcludes =
-      "**/test/**,"
-          + "**/testFixtures/**,"
-          + "**/*Test.java,"
-          + "**/intTest/**,"
-          + "**/tests/**,"
-          + "**/target/**,"
-          + "**/build/**,"
-          + "**/.mvn/**,"
-          + ".mvn/**";
-
   @Test
   void it_injects_dependency_even_when_no_poms_included() throws Exception {
 
@@ -125,13 +101,20 @@ void it_transforms_webgoat_normally() throws Exception {
         report.getResults().stream()
             .map(CodeTFResult::getChangeset)
             .flatMap(Collection::stream)
-            .collect(Collectors.toList());
+            .toList();
 
     assertThat(fileChanges.size(), is(58));
 
     // we only inject into a couple files
     verifyStandardCodemodResults(fileChanges);
 
+    // and that we inject the correct pom
+    assertThat(
+        fileChanges.stream()
+            .map(CodeTFChangesetEntry::getPath)
+            .anyMatch(path -> path.endsWith("insecure-deserialization/pom.xml")),
+        is(true));
+
     // this file is only changed by including the codeql results, which we didn't do in this test
     assertThat(
         fileChanges.stream()
@@ -165,9 +148,9 @@ void it_transforms_webgoat_with_codeql() throws Exception {
         report.getResults().stream()
             .map(CodeTFResult::getChangeset)
             .flatMap(Collection::stream)
-            .collect(Collectors.toList());
+            .toList();
 
-    assertThat(fileChanges.size(), is(62));
+    assertThat(fileChanges.size(), is(64));
 
     verifyStandardCodemodResults(fileChanges);
 
@@ -187,37 +170,10 @@ void it_transforms_webgoat_with_codeql() throws Exception {
 
     assertThat(ajaxJwtChange.getChanges().size(), equalTo(1));
     assertThat(ajaxJwtChange.getChanges().get(0).getLineNumber(), equalTo(53));
-  }
-
-  private static void verifyStandardCodemodResults(final List<CodeTFChangesetEntry> fileChanges) {
-    // we only inject into a couple files
-    assertThat(
-        fileChanges.stream()
-            .map(CodeTFChangesetEntry::getPath)
-            .anyMatch(path -> path.endsWith("SerializationHelper.java")),
-        is(true));
 
-    assertThat(
-        fileChanges.stream()
-            .map(CodeTFChangesetEntry::getPath)
-            .anyMatch(path -> path.endsWith("InsecureDeserializationTask.java")),
-        is(true));
-
-    // and inject the correct pom
-
-    assertThat(
-        fileChanges.stream()
-            .map(CodeTFChangesetEntry::getPath)
-            .anyMatch(path -> path.equals("webgoat-lessons/insecure-deserialization/pom.xml")),
-        is(true));
-  }
-
-  private static void verifyNoFailedFiles(final CodeTFReport report) {
-    List<String> failedFiles =
-        report.getResults().stream()
-            .map(CodeTFResult::getFailedFiles)
-            .flatMap(Collection::stream)
-            .toList();
-    assertThat(failedFiles.size(), is(0));
+    verifyCodemodsHitWithChangesetCount(report, "codeql:java/insecure-randomness", 0);
+    verifyCodemodsHitWithChangesetCount(report, "codeql:java/ssrf", 3);
+    verifyCodemodsHitWithChangesetCount(report, "codeql:java/sql-injection", 5);
+    verifyCodemodsHitWithChangesetCount(report, "codeql:java/insecure-cookie", 1);
   }
 }

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

@@ -2,6 +2,7 @@
 
 import io.codemodder.CodeChanger;
 import io.codemodder.Runner;
+import io.codemodder.codemods.codeql.*;
 import io.codemodder.codemods.semgrep.SemgrepJavaDeserializationCodemod;
 import io.codemodder.codemods.semgrep.SemgrepMissingSecureFlagCodemod;
 import io.codemodder.codemods.semgrep.SemgrepReflectionInjectionCodemod;
@@ -25,6 +26,21 @@ public static List<Class<? extends CodeChanger>> asList() {
         AddClarifyingBracesCodemod.class,
         AddMissingOverrideCodemod.class,
         AvoidImplicitPublicConstructorCodemod.class,
+        CodeQLDeserializationOfUserControlledDataCodemod.class,
+        CodeQLHttpResponseSplittingCodemod.class,
+        CodeQLInputResourceLeakCodemod.class,
+        CodeQLInsecureCookieCodemod.class,
+        CodeQLInsecureRandomnessCodemod.class,
+        CodeQLJDBCResourceLeakCodemod.class,
+        CodeQLJEXLInjectionCodemod.class,
+        CodeQLJNDIInjectionCodemod.class,
+        CodeQLMavenSecureURLCodemod.class,
+        CodeQLOutputResourceLeakCodemod.class,
+        CodeQLSQLInjectionCodemod.class,
+        CodeQLSSRFCodemod.class,
+        CodeQLStackTraceExposureCodemod.class,
+        CodeQLUnverifiedJwtCodemod.class,
+        CodeQLXXECodemod.class,
         DeclareVariableOnSeparateLineCodemod.class,
         DefectDojoSqlInjectionCodemod.class,
         DefineConstantForLiteralCodemod.class,
@@ -39,14 +55,8 @@ public static List<Class<? extends CodeChanger>> asList() {
         HardenXStreamCodemod.class,
         HardenZipEntryPathsCodemod.class,
         HQLParameterizationCodemod.class,
-        InputResourceLeakCodemod.class,
-        InsecureCookieCodemod.class,
-        JDBCResourceLeakCodemod.class,
-        JEXLInjectionCodemod.class,
         JSPScriptletXSSCodemod.class,
         LimitReadlineCodemod.class,
-        MavenSecureURLCodemod.class,
-        OutputResourceLeakCodemod.class,
         OverridesMatchParentSynchronizationCodemod.class,
         PreventFileWriterLeakWithFilesCodemod.class,
         RandomizeSeedCodemod.class,
@@ -83,10 +93,8 @@ public static List<Class<? extends CodeChanger>> asList() {
         SonarXXECodemod.class,
         SQLParameterizerCodemod.class,
         SSRFCodemod.class,
-        StackTraceExposureCodemod.class,
         SwitchLiteralFirstComparisonsCodemod.class,
         SwitchToStandardCharsetsCodemod.class,
-        UnverifiedJwtCodemod.class,
         UpgradeSSLContextTLSCodemod.class,
         UpgradeSSLEngineTLSCodemod.class,
         UpgradeSSLParametersTLSCodemod.class,

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

@@ -13,6 +13,7 @@
 import io.codemodder.*;
 import io.codemodder.javaparser.ChangesResult;
 import io.codemodder.providers.sarif.semgrep.SemgrepScan;
+import io.codemodder.remediation.resourceleak.ResourceLeakFixer;
 import java.io.File;
 import java.io.Writer;
 import javax.inject.Inject;

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

@@ -4,6 +4,7 @@
 import com.github.javaparser.ast.expr.Expression;
 import io.codemodder.*;
 import io.codemodder.javaparser.JavaParserChanger;
+import io.codemodder.remediation.resourceleak.ResourceLeakFixer;
 import java.util.List;
 import java.util.Optional;
 import java.util.stream.Collectors;

core-codemods/src/main/java/io/codemodder/codemods/codeql/CodeQLDeserializationOfUserControlledDataCodemod.java

@@ -0,0 +1,50 @@
+package io.codemodder.codemods.codeql;
+
+import com.github.javaparser.ast.CompilationUnit;
+import io.codemodder.*;
+import io.codemodder.codetf.DetectorRule;
+import io.codemodder.providers.sarif.codeql.ProvidedCodeQLScan;
+import io.codemodder.remediation.GenericRemediationMetadata;
+import io.codemodder.remediation.javadeserialization.JavaDeserializationRemediator;
+import javax.inject.Inject;
+
+/** A codemod for automatically fixing untrusted deserialization from CodeQL. */
+@Codemod(
+    id = "codeql:java/unsafe-deserialization",
+    reviewGuidance = ReviewGuidance.MERGE_WITHOUT_REVIEW,
+    importance = Importance.HIGH,
+    executionPriority = CodemodExecutionPriority.HIGH)
+public final class CodeQLDeserializationOfUserControlledDataCodemod
+    extends CodeQLRemediationCodemod {
+
+  private final JavaDeserializationRemediator remediator;
+
+  @Inject
+  public CodeQLDeserializationOfUserControlledDataCodemod(
+      @ProvidedCodeQLScan(ruleId = "java/unsafe-deserialization") final RuleSarif sarif) {
+    super(GenericRemediationMetadata.DESERIALIZATION.reporter(), sarif);
+    this.remediator = JavaDeserializationRemediator.DEFAULT;
+  }
+
+  @Override
+  public DetectorRule detectorRule() {
+    return new DetectorRule(
+        "unsafe-deserialization",
+        "Deserialization of user-controlled data",
+        "https://codeql.github.com/codeql-query-help/java/java-unsafe-deserialization/");
+  }
+
+  @Override
+  public CodemodFileScanningResult visit(
+      final CodemodInvocationContext context, final CompilationUnit cu) {
+    return remediator.remediateAll(
+        cu,
+        context.path().toString(),
+        detectorRule(),
+        ruleSarif.getResultsByLocationPath(context.path()),
+        SarifFindingKeyUtil::buildFindingId,
+        r -> r.getLocations().get(0).getPhysicalLocation().getRegion().getStartLine(),
+        r -> r.getLocations().get(0).getPhysicalLocation().getRegion().getEndLine(),
+        r -> r.getLocations().get(0).getPhysicalLocation().getRegion().getStartColumn());
+  }
+}