Skip to content

Commit

Permalink
First case of Sonar SSRF codemod + tests
Browse files Browse the repository at this point in the history
  • Loading branch information
andrecsilva committed Sep 24, 2024
1 parent 9196cb2 commit a6002f2
Show file tree
Hide file tree
Showing 7 changed files with 670 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ public static List<Class<? extends CodeChanger>> asList() {
SonarXXECodemod.class,
SonarSQLInjectionCodemod.class,
SonarUnsafeReflectionRemediationCodemod.class,
SonarSSRFCodemod.class,
SQLParameterizerCodemod.class,
SSRFCodemod.class,
StackTraceExposureCodemod.class,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
package io.codemodder.codemods;

import com.github.javaparser.ast.CompilationUnit;
import io.codemodder.*;
import io.codemodder.codemods.remediators.ssrf.SSRFRemediator;
import io.codemodder.codemods.remediators.ssrf.WhitelistSSRFRemediator;
import io.codemodder.codetf.DetectorRule;
import io.codemodder.providers.sonar.ProvidedSonarScan;
import io.codemodder.providers.sonar.RuleIssue;
import io.codemodder.providers.sonar.SonarRemediatingJavaParserChanger;
import io.codemodder.remediation.GenericRemediationMetadata;
import io.codemodder.sonar.model.Issue;
import io.codemodder.sonar.model.SonarFinding;
import java.util.List;
import java.util.Objects;
import javax.inject.Inject;

/**
* Fixes some Semgrep issues reported under the id
* "java.spring.security.injection.tainted-url-host.tainted-url-host"
*/
@Codemod(
id = "sonar:java/ssrf-s5144",
reviewGuidance = ReviewGuidance.MERGE_AFTER_CURSORY_REVIEW,
executionPriority = CodemodExecutionPriority.HIGH,
importance = Importance.HIGH)
public final class SonarSSRFCodemod extends SonarRemediatingJavaParserChanger {

private final SSRFRemediator remediator;
private final RuleIssue issues;

@Inject
public SonarSSRFCodemod(
@ProvidedSonarScan(ruleId = "javasecurity:S5144") final RuleIssue issues) {
super(GenericRemediationMetadata.SSRF.reporter(), issues);
this.issues = Objects.requireNonNull(issues);
this.remediator = new WhitelistSSRFRemediator();
}

@Override
public DetectorRule detectorRule() {
return new DetectorRule(
"javasecurity:S5144",
"Server-side requests should not be vulnerable to forging attacks",
"https://rules.sonarsource.com/java/RSPEC-5144/");
}

@Override
public CodemodFileScanningResult visit(
final CodemodInvocationContext context, final CompilationUnit cu) {
List<Issue> issuesForFile = issues.getResultsByPath(context.path());
return remediator.remediateAll(
cu,
context.path().toString(),
detectorRule(),
issuesForFile,
SonarFinding::getKey,
i -> i.getTextRange() != null ? i.getTextRange().getStartLine() : i.getLine(),
i -> i.getTextRange() != null ? i.getTextRange().getEndLine() : null,
i -> i.getTextRange() != null ? i.getTextRange().getStartOffset() : null);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,199 @@
package io.codemodder.codemods.remediators.ssrf;

import com.github.javaparser.StaticJavaParser;
import com.github.javaparser.ast.CompilationUnit;
import com.github.javaparser.ast.body.CallableDeclaration;
import com.github.javaparser.ast.body.ClassOrInterfaceDeclaration;
import com.github.javaparser.ast.expr.*;
import io.codemodder.CodemodChange;
import io.codemodder.CodemodFileScanningResult;
import io.codemodder.codetf.DetectorRule;
import io.codemodder.codetf.FixedFinding;
import io.codemodder.codetf.UnfixedFinding;
import io.codemodder.remediation.FixCandidate;
import io.codemodder.remediation.FixCandidateSearchResults;
import io.codemodder.remediation.FixCandidateSearcher;
import io.codemodder.remediation.MethodOrConstructor;
import java.util.ArrayList;
import java.util.List;
import java.util.Random;
import java.util.function.BiPredicate;
import java.util.function.Function;
import java.util.stream.Collectors;
import org.javatuples.Pair;

public final class WhitelistSSRFRemediator implements SSRFRemediator {

@Override
public <T> CodemodFileScanningResult remediateAll(
final CompilationUnit cu,
final String path,
final DetectorRule detectorRule,
final List<T> issuesForFile,
final Function<T, String> getKey,
final Function<T, Integer> getStartLine,
final Function<T, Integer> getEndLine,
final Function<T, Integer> getStartColumn) {

List<CodemodChange> changes = new ArrayList<>();
List<UnfixedFinding> unfixedFindings = new ArrayList<>();

// new URL(url) case
FixCandidateSearcher<T> urlSearcher =
new FixCandidateSearcher.Builder<T>()
.withMatcher(mce -> mce.isConstructorForType("URL"))
.withMatcher(mce -> !mce.getArguments().isEmpty())
.build();

// ...

// RestTemplate().exchange(url,...)
FixCandidateSearcher<T> rtSearcher =
new FixCandidateSearcher.Builder<T>()
// is method with name
.withMatcher(mce -> mce.isMethodCallWithName("exchange"))
// has RestTemplate as scope
.withMatcher(MethodOrConstructor::isMethodCallWithScope)
// .withMatcher(mce -> mce.asMethodCall().getScope().filter(s ->
// (("org.springframework.web.client" +
// ".RestTemplate").equals(s.calculateResolvedType().describe()))).isPresent())
.build();

var pairResult =
searchAndFix(
rtSearcher,
(cunit, moc) -> hardenRT(cunit, moc.asMethodCall()),
cu,
path,
detectorRule,
issuesForFile,
getKey,
getStartLine,
getEndLine,
getStartColumn);
changes.addAll(pairResult.getValue0());
unfixedFindings.addAll(pairResult.getValue1());

return CodemodFileScanningResult.from(changes, unfixedFindings);
}

private <T> Pair<List<CodemodChange>, List<UnfixedFinding>> searchAndFix(
final FixCandidateSearcher<T> searcher,
final BiPredicate<CompilationUnit, MethodOrConstructor> fixer,
final CompilationUnit cu,
final String path,
final DetectorRule detectorRule,
final List<T> issuesForFile,
final Function<T, String> getKey,
final Function<T, Integer> getStartLine,
final Function<T, Integer> getEndLine,
final Function<T, Integer> getStartColumn) {
List<CodemodChange> changes = new ArrayList<>();
List<UnfixedFinding> unfixedFindings = new ArrayList<>();

FixCandidateSearchResults<T> results =
searcher.search(
cu,
path,
detectorRule,
issuesForFile,
getKey,
getStartLine,
getEndLine,
getStartColumn);

for (FixCandidate<T> candidate : results.fixCandidates()) {
MethodOrConstructor call = candidate.call();
List<T> issues = candidate.issues();
if (fixer.test(cu, call)) {
List<FixedFinding> fixedFindings =
issues.stream()
.map(issue -> new FixedFinding(getKey.apply(issue), detectorRule))
.toList();
CodemodChange change =
CodemodChange.from(getStartLine.apply(issues.get(0)), List.of(), fixedFindings);
changes.add(change);
} else {
issues.forEach(
issue -> {
final String id = getKey.apply(issue);
final UnfixedFinding unfixableFinding =
new UnfixedFinding(
id,
detectorRule,
path,
getStartLine.apply(issues.get(0)),
"State changing effects possible or unrecognized code shape");
unfixedFindings.add(unfixableFinding);
});
}
}
return Pair.with(changes, unfixedFindings);
}

private boolean hardenURL(final CompilationUnit cu, final ObjectCreationExpr newUrlCreation) {
var classDecl = newUrlCreation.findAncestor(ClassOrInterfaceDeclaration.class);
return false;
}

static final String defaultMethodName = "filterURL";

private static String generateFilterMethodName(final ClassOrInterfaceDeclaration classDecl) {
var methodNames =
classDecl.getMethods().stream()
.map(CallableDeclaration::getNameAsString)
.filter(s -> s.startsWith(defaultMethodName))
.sorted()
.collect(Collectors.toCollection(ArrayList::new));
if (methodNames.isEmpty()) {
return defaultMethodName;
}
String number = methodNames.get(methodNames.size() - 1).substring(defaultMethodName.length());
if (number.isBlank()) {
return defaultMethodName + "_1";
}
int num = (new Random()).nextInt();
try {
num = Integer.parseInt(number.substring(1)) + 1;
} catch (NumberFormatException e) {
}
return defaultMethodName + "_" + num;
}

private static void addFilterMethod(
final ClassOrInterfaceDeclaration classDecl, final String newMethodName) {
final String method =
"""
String %s(final String url){
var allowedHosts = List.of("");
if (!allowedHosts.contains(url)){
throw new SecurityException("Supplied URL is not allowed.");
}
return url;
}
"""
.formatted(newMethodName);
classDecl.addMember(StaticJavaParser.parseMethodDeclaration(method));
}

private boolean hardenRT(final CompilationUnit cu, final MethodCallExpr call) {
var maybeFirstArg =
call.getArguments().stream()
.findFirst()
.filter(
arg ->
!(arg.isMethodCallExpr()
&& arg.asMethodCallExpr().getNameAsString().startsWith(defaultMethodName)));
if (maybeFirstArg.isPresent()) {
var maybeClassDecl = call.findAncestor(ClassOrInterfaceDeclaration.class);
if (maybeClassDecl.isPresent()) {
var newMethodName = generateFilterMethodName(maybeClassDecl.get());
addFilterMethod(maybeClassDecl.get(), newMethodName);
var wrappedArg = new MethodCallExpr(newMethodName, maybeFirstArg.get().clone());
maybeFirstArg.get().replace(wrappedArg);
return true;
}
}
return false;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package io.codemodder.codemods;

import io.codemodder.testutils.CodemodTestMixin;
import io.codemodder.testutils.Metadata;
import org.junit.jupiter.api.Nested;

final class SonarSSRFCodemodTest {

@Nested
@Metadata(
codemodType = SonarSSRFCodemod.class,
testResourceDir = "sonar-ssrf-s5144/resttemplate",
renameTestFile =
"src/main/java/org/owasp/webgoat/lessons/passwordreset/ResetLinkAssignmentForgotPassword.java",
expectingFixesAtLines = {104},
dependencies = {})
class RestTemplateTest implements CodemodTestMixin {}
}
Loading

0 comments on commit a6002f2

Please sign in to comment.