Added path traversal remediation (#493)

The scope of this is limited for a first introduction, since path
traversal will be a tricky one to generalize more. This change
introduces a remediator that will sanitize PT flows that start with an
obvious source of taint that is intended to be a filename -- multipart
file names.

Author: nahsra

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

@@ -20,6 +20,7 @@ public enum GenericRemediationMetadata {
   REGEX_INJECTION("regex-injection"),
   ERROR_MESSAGE_EXPOSURE("error-message-exposure"),
   LOG_INJECTION("log-injection"),
+  PATH_TRAVERSAL("path-traversal"),
   WEAK_CRYPTO_ALGORITHM("weak-crypto-algorithm");
 
   private final CodemodReporterStrategy reporter;

framework/codemodder-base/src/main/java/io/codemodder/remediation/pathtraversal/PathTraversalRemediator.java

@@ -0,0 +1,60 @@
+package io.codemodder.remediation.pathtraversal;
+
+import com.github.javaparser.ast.CompilationUnit;
+import com.github.javaparser.ast.expr.MethodCallExpr;
+import io.codemodder.CodemodFileScanningResult;
+import io.codemodder.codetf.DetectorRule;
+import io.codemodder.remediation.FixCandidateSearcher;
+import io.codemodder.remediation.Remediator;
+import io.codemodder.remediation.SearcherStrategyRemediator;
+import java.util.Collection;
+import java.util.Optional;
+import java.util.function.Function;
+
+/** Remediate path traversal vulns. */
+public final class PathTraversalRemediator<T> implements Remediator<T> {
+
+  private final SearcherStrategyRemediator<T> searchStrategyRemediator;
+
+  public PathTraversalRemediator() {
+    this.searchStrategyRemediator =
+        new SearcherStrategyRemediator.Builder<T>()
+            .withSearcherStrategyPair(
+                new FixCandidateSearcher.Builder<T>()
+                    .withMatcher(
+                        node ->
+                            Optional.of(node)
+                                .map(n -> n instanceof MethodCallExpr ? (MethodCallExpr) n : null)
+                                .filter(PathTraversalRemediator::isSpringMultipartFilenameCall)
+                                .isPresent())
+                    .build(),
+                new SpringMultipartFixStrategy())
+            .build();
+  }
+
+  @Override
+  public CodemodFileScanningResult remediateAll(
+      final CompilationUnit cu,
+      final String path,
+      final DetectorRule detectorRule,
+      final Collection<T> findingsForPath,
+      final Function<T, String> findingIdExtractor,
+      final Function<T, Integer> findingStartLineExtractor,
+      final Function<T, Optional<Integer>> findingEndLineExtractor,
+      final Function<T, Optional<Integer>> findingStartColumnExtractor) {
+    return searchStrategyRemediator.remediateAll(
+        cu,
+        path,
+        detectorRule,
+        findingsForPath,
+        findingIdExtractor,
+        findingStartLineExtractor,
+        findingEndLineExtractor,
+        findingStartColumnExtractor);
+  }
+
+  private static boolean isSpringMultipartFilenameCall(final MethodCallExpr methodCallExpr) {
+    return methodCallExpr.hasScope()
+        && "getOriginalFilename".equals(methodCallExpr.getNameAsString());
+  }
+}

framework/codemodder-base/src/main/java/io/codemodder/remediation/pathtraversal/SpringMultipartFixStrategy.java

@@ -0,0 +1,24 @@
+package io.codemodder.remediation.pathtraversal;
+
+import static io.codemodder.javaparser.JavaParserTransformer.wrap;
+
+import com.github.javaparser.ast.CompilationUnit;
+import com.github.javaparser.ast.Node;
+import com.github.javaparser.ast.expr.MethodCallExpr;
+import io.codemodder.remediation.RemediationStrategy;
+import io.codemodder.remediation.SuccessOrReason;
+import io.github.pixee.security.Filenames;
+
+/**
+ * Fix strategy for Spring MultipartFile getOriginalFilename() calls which wraps with
+ * java-security-toolkit API for guaranteeing a simple file name.
+ */
+final class SpringMultipartFixStrategy implements RemediationStrategy {
+  @Override
+  public SuccessOrReason fix(final CompilationUnit cu, final Node node) {
+    MethodCallExpr methodCallExpr = (MethodCallExpr) node;
+    boolean success =
+        wrap(methodCallExpr).withStaticMethod(Filenames.class.getName(), "toSimpleFileName", false);
+    return success ? SuccessOrReason.success() : SuccessOrReason.reason("Wrap failed");
+  }
+}

framework/codemodder-base/src/main/resources/generic-remediation-reports/path-traversal/description.md

@@ -0,0 +1,13 @@
+This change attempts to remediate path traversal (also called directory traversal, local file include, etc.) vulnerabilities.
+
+Our changes may introduce sanitization up front, if the input looks like it's a file name (like from a multipart HTTP request), or validation if it appears to be a piece of path.
+
+
+```diff
++ import io.github.pixee.security.Filenames;
+
+  ...
+  
+- String fileName = request.getFileName();
++ String fileName = Filenames.toSimpleFileName(request.getFileName());
+```

framework/codemodder-base/src/main/resources/generic-remediation-reports/path-traversal/report.json

@@ -0,0 +1,8 @@
+{
+  "summary" : "Remediate path traversal",
+  "change": "Added a validator/sanitizer",
+  "reviewGuidanceIJustification" : "These changes should be reviewed to ensure that the validator/sanitizer is correctly implemented and won't disrupt the application's functionality.",
+  "references" : [
+    "https://cwe.mitre.org/data/definitions/35.html"
+  ]
+}

framework/codemodder-base/src/test/java/io/codemodder/RemediatorTestMixin.java

@@ -0,0 +1,89 @@
+package io.codemodder;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import com.github.javaparser.StaticJavaParser;
+import com.github.javaparser.ast.CompilationUnit;
+import com.github.javaparser.printer.lexicalpreservation.LexicalPreservingPrinter;
+import io.codemodder.codetf.DetectorRule;
+import io.codemodder.remediation.Remediator;
+import java.util.List;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.stream.Stream;
+import org.junit.jupiter.api.DynamicTest;
+import org.junit.jupiter.api.TestFactory;
+
+/** A mixin which provides basic structure for testing remediators. */
+public interface RemediatorTestMixin<T> {
+
+  @TestFactory
+  default Stream<DynamicTest> it_fixes_the_code() {
+    Stream<FixableSample> fixableSamples = createFixableSamples();
+
+    return fixableSamples.map(
+        sample -> {
+          String beforeCode = sample.beforeCode();
+          String afterCode = sample.afterCode();
+          int line = sample.line();
+
+          return DynamicTest.dynamicTest(
+              beforeCode,
+              () -> {
+                CompilationUnit cu = StaticJavaParser.parse(beforeCode);
+                LexicalPreservingPrinter.setup(cu);
+                Remediator<Object> remediator = createRemediator();
+
+                DetectorRule rule = new DetectorRule("rule-123", "my-remediation-rule", null);
+
+                CodemodFileScanningResult result =
+                    remediator.remediateAll(
+                        cu,
+                        "foo",
+                        rule,
+                        List.of(new Object()),
+                        f -> "123",
+                        f -> line,
+                        x -> Optional.empty(),
+                        x -> Optional.empty());
+                assertThat(result.unfixedFindings()).isEmpty();
+                assertThat(result.changes()).hasSize(1);
+                CodemodChange change = result.changes().get(0);
+                assertThat(change.lineNumber()).isEqualTo(line);
+
+                String actualCode = LexicalPreservingPrinter.print(cu);
+                assertThat(actualCode).isEqualToIgnoringCase(afterCode);
+              });
+        });
+  }
+
+  /** Build the remediator to test. */
+  Remediator<Object> createRemediator();
+
+  /** Create samples to test. */
+  Stream<FixableSample> createFixableSamples();
+
+  /** Create unfixable samples. */
+  Stream<UnfixableSample> createUnfixableSamples();
+
+  /** Represents a finding that can be fixed. */
+  record FixableSample(String beforeCode, String afterCode, int line) {
+    public FixableSample {
+      Objects.requireNonNull(beforeCode);
+      Objects.requireNonNull(afterCode);
+      if (line < 0) {
+        throw new IllegalArgumentException("Line number must be non-negative");
+      }
+    }
+  }
+
+  /** Represents a finding that can't be fixed for some reason. */
+  record UnfixableSample(String code, int line) {
+    public UnfixableSample {
+      Objects.requireNonNull(code);
+      if (line < 0) {
+        throw new IllegalArgumentException("Line number must be non-negative");
+      }
+    }
+  }
+}

framework/codemodder-base/src/test/java/io/codemodder/remediation/pathtraversal/PathTraversalRemediatorTest.java

@@ -0,0 +1,72 @@
+package io.codemodder.remediation.pathtraversal;
+
+import io.codemodder.RemediatorTestMixin;
+import io.codemodder.remediation.Remediator;
+import java.util.stream.Stream;
+
+final class PathTraversalRemediatorTest implements RemediatorTestMixin<Object> {
+
+  @Override
+  public Remediator<Object> createRemediator() {
+    return new PathTraversalRemediator<>();
+  }
+
+  @Override
+  public Stream<FixableSample> createFixableSamples() {
+    return Stream.of(
+        new FixableSample(
+            """
+                        import org.springframework.web.multipart.MultipartFile;
+                        public class Test {
+                            public void test(MultipartFile file) {
+                                String filename = file.getOriginalFilename();
+                            }
+                        }
+                        """,
+            """
+                        import io.github.pixee.security.Filenames;
+                        import org.springframework.web.multipart.MultipartFile;
+                        public class Test {
+                            public void test(MultipartFile file) {
+                                String filename = Filenames.toSimpleFilename(file.getOriginalFilename());
+                            }
+                        }
+                        """,
+            4),
+        new FixableSample(
+            """
+                        import org.springframework.web.multipart.MultipartFile;
+                        public class Test {
+                            public void test(MultipartFile file) {
+                                String filename = new File(dir, file.getOriginalFilename());
+                            }
+                        }
+                        """,
+            """
+                        import io.github.pixee.security.Filenames;
+                        import org.springframework.web.multipart.MultipartFile;
+                        public class Test {
+                            public void test(MultipartFile file) {
+                                String filename = new File(dir, Filenames.toSimpleFilename(file.getOriginalFilename()));
+                            }
+                        }
+                        """,
+            4));
+  }
+
+  @Override
+  public Stream<UnfixableSample> createUnfixableSamples() {
+    return Stream.of(
+        // no getOriginalFilename() call
+        new UnfixableSample(
+            """
+                                import org.springframework.web.multipart.MultipartFile;
+                                public class Test {
+                                    public void test(MultipartFile file) {
+                                        String filename = file.filename();
+                                    }
+                                }
+                                """,
+            4));
+  }
+}