Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
@@ -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());
}
}
Original file line number Diff line number Diff line change
@@ -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");
}
}
Original file line number Diff line number Diff line change
@@ -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());
```
Original file line number Diff line number Diff line change
@@ -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"
]
}
Original file line number Diff line number Diff line change
@@ -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");
}
}
}
}
Original file line number Diff line number Diff line change
@@ -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));
}
}
Loading