From 0a7867c89d7d84e2c07331bb1af664e6a9e7d984 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20C=2E=20Silva?= <12188364+andrecsilva@users.noreply.github.com> Date: Mon, 21 Oct 2024 09:56:19 -0300 Subject: [PATCH] Converted some remediators to new API (#460) Co-authored-by: pixeebot[bot] <104101892+pixeebot[bot]@users.noreply.github.com> Co-authored-by: pixeebot[bot] --- .../DefectDojoSqlInjectionCodemod.java | 6 +- .../SonarObjectDeserializationCodemod.java | 11 +- .../codemods/SonarSQLInjectionCodemod.java | 27 ++- .../codemodder/codemods/SonarSSRFCodemod.java | 11 +- ...narUnsafeReflectionRemediationCodemod.java | 11 +- ...ializationOfUserControlledDataCodemod.java | 13 +- .../CodeQLHttpResponseSplittingCodemod.java | 13 +- .../codeql/CodeQLSQLInjectionCodemod.java | 27 ++- .../codemods/codeql/CodeQLSSRFCodemod.java | 13 +- .../SemgrepJavaDeserializationCodemod.java | 15 +- .../SemgrepMissingSecureFlagCodemod.java | 15 +- .../SemgrepReflectionInjectionCodemod.java | 15 +- .../semgrep/SemgrepSQLInjectionCodemod.java | 18 +- .../codemods/semgrep/SemgrepSSRFCodemod.java | 15 +- .../src/main/java/io/codemodder/Either.java | 29 +-- .../DefaultFixCandidateSearcher.java | 12 +- .../DefaultHeaderInjectionRemediator.java | 120 ---------- .../HeaderInjectionFixStrategy.java | 75 +++++++ .../HeaderInjectionRemediator.java | 64 ++++-- .../DefaultJavaDeserializationRemediator.java | 194 ---------------- .../JavaDeserializationFixStrategy.java | 103 +++++++++ .../JavaDeserializationRemediator.java | 82 +++++-- .../DefaultMissingSecureFlagRemediator.java | 126 ----------- .../MissingSecureFlagFixStrategy.java | 64 ++++++ .../MissingSecureFlagRemediator.java | 53 ++++- .../DefaultReflectionInjectionRemediator.java | 130 ----------- .../ReflectionInjectionFixStrategy.java | 46 ++++ .../ReflectionInjectionRemediator.java | 89 +++++++- .../sqlinjection/SQLInjectionFixComposer.java | 6 +- .../sqlinjection/SQLInjectionRemediator.java | 6 +- .../ssrf/DefaultSSRFRemediator.java | 211 ------------------ .../remediation/ssrf/SSRFFixStrategy.java | 95 ++++++++ .../remediation/ssrf/SSRFRemediator.java | 74 +++++- .../DefaultHeaderInjectionRemediatorTest.java | 13 +- ...aultJavaDeserializationRemediatorTest.java | 23 +- ...efaultMissingSecureFlagRemediatorTest.java | 13 +- ...aultReflectionInjectionRemediatorTest.java | 17 +- .../sarif/appscan/AppScanRuleSarif.java | 2 +- 38 files changed, 917 insertions(+), 940 deletions(-) delete mode 100644 framework/codemodder-base/src/main/java/io/codemodder/remediation/headerinjection/DefaultHeaderInjectionRemediator.java create mode 100644 framework/codemodder-base/src/main/java/io/codemodder/remediation/headerinjection/HeaderInjectionFixStrategy.java delete mode 100644 framework/codemodder-base/src/main/java/io/codemodder/remediation/javadeserialization/DefaultJavaDeserializationRemediator.java create mode 100644 framework/codemodder-base/src/main/java/io/codemodder/remediation/javadeserialization/JavaDeserializationFixStrategy.java delete mode 100644 framework/codemodder-base/src/main/java/io/codemodder/remediation/missingsecureflag/DefaultMissingSecureFlagRemediator.java create mode 100644 framework/codemodder-base/src/main/java/io/codemodder/remediation/missingsecureflag/MissingSecureFlagFixStrategy.java delete mode 100644 framework/codemodder-base/src/main/java/io/codemodder/remediation/reflectioninjection/DefaultReflectionInjectionRemediator.java create mode 100644 framework/codemodder-base/src/main/java/io/codemodder/remediation/reflectioninjection/ReflectionInjectionFixStrategy.java delete mode 100644 framework/codemodder-base/src/main/java/io/codemodder/remediation/ssrf/DefaultSSRFRemediator.java create mode 100644 framework/codemodder-base/src/main/java/io/codemodder/remediation/ssrf/SSRFFixStrategy.java diff --git a/core-codemods/src/main/java/io/codemodder/codemods/DefectDojoSqlInjectionCodemod.java b/core-codemods/src/main/java/io/codemodder/codemods/DefectDojoSqlInjectionCodemod.java index 81f49df00..15258c954 100644 --- a/core-codemods/src/main/java/io/codemodder/codemods/DefectDojoSqlInjectionCodemod.java +++ b/core-codemods/src/main/java/io/codemodder/codemods/DefectDojoSqlInjectionCodemod.java @@ -27,7 +27,7 @@ public final class DefectDojoSqlInjectionCodemod extends JavaParserChanger implements FixOnlyCodeChanger { private final RuleFindings findings; - private final Remediator remediatorStrategy; + private final Remediator remediationStrategy; @Inject public DefectDojoSqlInjectionCodemod( @@ -35,7 +35,7 @@ public DefectDojoSqlInjectionCodemod( RuleFindings findings) { super(CodemodReporterStrategy.fromClasspath(SQLParameterizerCodemod.class)); this.findings = Objects.requireNonNull(findings); - this.remediatorStrategy = new SQLInjectionRemediator<>(); + this.remediationStrategy = new SQLInjectionRemediator<>(); } @Override @@ -55,7 +55,7 @@ public DetectorRule detectorRule() { public CodemodFileScanningResult visit( final CodemodInvocationContext context, final CompilationUnit cu) { List findingsForThisPath = findings.getForPath(context.path()); - return remediatorStrategy.remediateAll( + return remediationStrategy.remediateAll( cu, context.path().toString(), detectorRule(), diff --git a/core-codemods/src/main/java/io/codemodder/codemods/SonarObjectDeserializationCodemod.java b/core-codemods/src/main/java/io/codemodder/codemods/SonarObjectDeserializationCodemod.java index 493c89f8e..acc9ef4f9 100644 --- a/core-codemods/src/main/java/io/codemodder/codemods/SonarObjectDeserializationCodemod.java +++ b/core-codemods/src/main/java/io/codemodder/codemods/SonarObjectDeserializationCodemod.java @@ -7,11 +7,14 @@ import io.codemodder.providers.sonar.RuleIssue; import io.codemodder.providers.sonar.SonarRemediatingJavaParserChanger; import io.codemodder.remediation.GenericRemediationMetadata; +import io.codemodder.remediation.Remediator; import io.codemodder.remediation.javadeserialization.JavaDeserializationRemediator; import io.codemodder.sonar.model.Issue; import io.codemodder.sonar.model.SonarFinding; +import io.codemodder.sonar.model.TextRange; import java.util.List; import java.util.Objects; +import java.util.Optional; import javax.inject.Inject; /** Fixes Object Deserialization issues found by sonar rule javasecurity:S5135. */ @@ -22,7 +25,7 @@ importance = Importance.HIGH) public final class SonarObjectDeserializationCodemod extends SonarRemediatingJavaParserChanger { - private final JavaDeserializationRemediator remediator; + private final Remediator remediator; private final RuleIssue issues; @Inject @@ -30,7 +33,7 @@ public SonarObjectDeserializationCodemod( @ProvidedSonarScan(ruleId = "javasecurity:S5135") final RuleIssue issues) { super(GenericRemediationMetadata.DESERIALIZATION.reporter(), issues); this.issues = Objects.requireNonNull(issues); - this.remediator = JavaDeserializationRemediator.DEFAULT; + this.remediator = new JavaDeserializationRemediator<>(); } @Override @@ -52,7 +55,7 @@ public CodemodFileScanningResult visit( 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); + i -> Optional.ofNullable(i.getTextRange()).map(TextRange::getEndLine), + i -> Optional.ofNullable(i.getTextRange()).map(tr -> tr.getStartOffset() + 1)); } } diff --git a/core-codemods/src/main/java/io/codemodder/codemods/SonarSQLInjectionCodemod.java b/core-codemods/src/main/java/io/codemodder/codemods/SonarSQLInjectionCodemod.java index c5027a89c..0baf3b4c7 100644 --- a/core-codemods/src/main/java/io/codemodder/codemods/SonarSQLInjectionCodemod.java +++ b/core-codemods/src/main/java/io/codemodder/codemods/SonarSQLInjectionCodemod.java @@ -1,14 +1,18 @@ package io.codemodder.codemods; import com.github.javaparser.ast.CompilationUnit; +import com.github.javaparser.ast.expr.Expression; import io.codemodder.*; +import io.codemodder.ast.ASTs; import io.codemodder.codetf.DetectorRule; import io.codemodder.providers.sonar.ProvidedSonarScan; import io.codemodder.providers.sonar.RuleHotspot; import io.codemodder.providers.sonar.SonarRemediatingJavaParserChanger; +import io.codemodder.remediation.FixCandidateSearcher; import io.codemodder.remediation.GenericRemediationMetadata; import io.codemodder.remediation.Remediator; -import io.codemodder.remediation.sqlinjection.SQLInjectionRemediator; +import io.codemodder.remediation.SearcherStrategyRemediator; +import io.codemodder.remediation.sqlinjection.SQLInjectionFixComposer; import io.codemodder.sonar.model.Hotspot; import io.codemodder.sonar.model.SonarFinding; import io.codemodder.sonar.model.TextRange; @@ -32,7 +36,26 @@ public SonarSQLInjectionCodemod( @ProvidedSonarScan(ruleId = "java:S2077") final RuleHotspot hotspots) { super(GenericRemediationMetadata.SQL_INJECTION.reporter(), hotspots); this.hotspots = Objects.requireNonNull(hotspots); - this.remediationStrategy = new SQLInjectionRemediator<>(); + this.remediationStrategy = + new SearcherStrategyRemediator.Builder() + .withSearcherStrategyPair( + new FixCandidateSearcher.Builder() + .withMatcher( + n -> + Optional.empty() + // is the argument of the call + .or( + () -> + Optional.of(n) + .map( + m -> + m instanceof Expression ? (Expression) m : null) + .flatMap(ASTs::isArgumentOfMethodCall) + .filter(SQLInjectionFixComposer::match)) + .isPresent()) + .build(), + new SQLInjectionFixComposer()) + .build(); } @Override diff --git a/core-codemods/src/main/java/io/codemodder/codemods/SonarSSRFCodemod.java b/core-codemods/src/main/java/io/codemodder/codemods/SonarSSRFCodemod.java index ab636fcaa..4a7f3b569 100644 --- a/core-codemods/src/main/java/io/codemodder/codemods/SonarSSRFCodemod.java +++ b/core-codemods/src/main/java/io/codemodder/codemods/SonarSSRFCodemod.java @@ -7,11 +7,14 @@ import io.codemodder.providers.sonar.RuleIssue; import io.codemodder.providers.sonar.SonarRemediatingJavaParserChanger; import io.codemodder.remediation.GenericRemediationMetadata; +import io.codemodder.remediation.Remediator; import io.codemodder.remediation.ssrf.SSRFRemediator; import io.codemodder.sonar.model.Issue; import io.codemodder.sonar.model.SonarFinding; +import io.codemodder.sonar.model.TextRange; import java.util.List; import java.util.Objects; +import java.util.Optional; import javax.inject.Inject; /** Fixes SSRF issues found by sonar rule javasecurity:S5144. */ @@ -22,7 +25,7 @@ importance = Importance.HIGH) public final class SonarSSRFCodemod extends SonarRemediatingJavaParserChanger { - private final SSRFRemediator remediator; + private final Remediator remediator; private final RuleIssue issues; @Inject @@ -30,7 +33,7 @@ public SonarSSRFCodemod( @ProvidedSonarScan(ruleId = "javasecurity:S5144") final RuleIssue issues) { super(GenericRemediationMetadata.SSRF.reporter(), issues); this.issues = Objects.requireNonNull(issues); - this.remediator = SSRFRemediator.DEFAULT; + this.remediator = new SSRFRemediator<>(); } @Override @@ -52,7 +55,7 @@ public CodemodFileScanningResult visit( 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); + i -> Optional.ofNullable(i.getTextRange()).map(TextRange::getEndLine), + i -> Optional.ofNullable(i.getTextRange()).map(tr -> tr.getStartOffset() + 1)); } } diff --git a/core-codemods/src/main/java/io/codemodder/codemods/SonarUnsafeReflectionRemediationCodemod.java b/core-codemods/src/main/java/io/codemodder/codemods/SonarUnsafeReflectionRemediationCodemod.java index 29ef707ed..cfc57adeb 100644 --- a/core-codemods/src/main/java/io/codemodder/codemods/SonarUnsafeReflectionRemediationCodemod.java +++ b/core-codemods/src/main/java/io/codemodder/codemods/SonarUnsafeReflectionRemediationCodemod.java @@ -7,9 +7,12 @@ import io.codemodder.providers.sonar.RuleIssue; import io.codemodder.providers.sonar.SonarRemediatingJavaParserChanger; import io.codemodder.remediation.GenericRemediationMetadata; +import io.codemodder.remediation.Remediator; import io.codemodder.remediation.reflectioninjection.ReflectionInjectionRemediator; import io.codemodder.sonar.model.Issue; +import io.codemodder.sonar.model.TextRange; import java.util.Objects; +import java.util.Optional; import javax.inject.Inject; /** Sonar remediation codemod for S2658: Classes should not be loaded dynamically. */ @@ -21,14 +24,14 @@ public final class SonarUnsafeReflectionRemediationCodemod extends SonarRemediatingJavaParserChanger { - private final ReflectionInjectionRemediator remediator; + private final Remediator remediator; private final RuleIssue issues; @Inject public SonarUnsafeReflectionRemediationCodemod( @ProvidedSonarScan(ruleId = "java:S2658") final RuleIssue issues) { super(GenericRemediationMetadata.REFLECTION_INJECTION.reporter(), issues); - this.remediator = ReflectionInjectionRemediator.DEFAULT; + this.remediator = new ReflectionInjectionRemediator<>(); this.issues = Objects.requireNonNull(issues); } @@ -50,7 +53,7 @@ public CodemodFileScanningResult visit( issues.getResultsByPath(context.path()), Issue::getKey, i -> i.getTextRange() != null ? i.getTextRange().getStartLine() : i.getLine(), - i -> i.getTextRange() != null ? i.getTextRange().getEndLine() : null, - i -> i.getTextRange().getStartOffset()); + i -> Optional.ofNullable(i.getTextRange()).map(TextRange::getEndLine), + i -> Optional.empty()); } } diff --git a/core-codemods/src/main/java/io/codemodder/codemods/codeql/CodeQLDeserializationOfUserControlledDataCodemod.java b/core-codemods/src/main/java/io/codemodder/codemods/codeql/CodeQLDeserializationOfUserControlledDataCodemod.java index 8f3845bb6..bcd4958d4 100644 --- a/core-codemods/src/main/java/io/codemodder/codemods/codeql/CodeQLDeserializationOfUserControlledDataCodemod.java +++ b/core-codemods/src/main/java/io/codemodder/codemods/codeql/CodeQLDeserializationOfUserControlledDataCodemod.java @@ -1,11 +1,14 @@ package io.codemodder.codemods.codeql; +import com.contrastsecurity.sarif.Result; 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.Remediator; import io.codemodder.remediation.javadeserialization.JavaDeserializationRemediator; +import java.util.Optional; import javax.inject.Inject; /** A codemod for automatically fixing untrusted deserialization from CodeQL. */ @@ -17,13 +20,13 @@ public final class CodeQLDeserializationOfUserControlledDataCodemod extends CodeQLRemediationCodemod { - private final JavaDeserializationRemediator remediator; + private final Remediator remediator; @Inject public CodeQLDeserializationOfUserControlledDataCodemod( @ProvidedCodeQLScan(ruleId = "java/unsafe-deserialization") final RuleSarif sarif) { super(GenericRemediationMetadata.DESERIALIZATION.reporter(), sarif); - this.remediator = JavaDeserializationRemediator.DEFAULT; + this.remediator = new JavaDeserializationRemediator<>(); } @Override @@ -44,7 +47,9 @@ public CodemodFileScanningResult visit( 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()); + r -> Optional.of(r.getLocations().get(0).getPhysicalLocation().getRegion().getEndLine()), + r -> + Optional.of( + r.getLocations().get(0).getPhysicalLocation().getRegion().getStartColumn())); } } diff --git a/core-codemods/src/main/java/io/codemodder/codemods/codeql/CodeQLHttpResponseSplittingCodemod.java b/core-codemods/src/main/java/io/codemodder/codemods/codeql/CodeQLHttpResponseSplittingCodemod.java index 790619b19..a1f7928d9 100644 --- a/core-codemods/src/main/java/io/codemodder/codemods/codeql/CodeQLHttpResponseSplittingCodemod.java +++ b/core-codemods/src/main/java/io/codemodder/codemods/codeql/CodeQLHttpResponseSplittingCodemod.java @@ -1,11 +1,14 @@ package io.codemodder.codemods.codeql; +import com.contrastsecurity.sarif.Result; 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.Remediator; import io.codemodder.remediation.headerinjection.HeaderInjectionRemediator; +import java.util.Optional; import javax.inject.Inject; /** A codemod for automatically fixing HTTP response splitting from CodeQL. */ @@ -16,13 +19,13 @@ executionPriority = CodemodExecutionPriority.HIGH) public final class CodeQLHttpResponseSplittingCodemod extends CodeQLRemediationCodemod { - private final HeaderInjectionRemediator remediator; + private final Remediator remediator; @Inject public CodeQLHttpResponseSplittingCodemod( @ProvidedCodeQLScan(ruleId = "java/http-response-splitting") final RuleSarif sarif) { super(GenericRemediationMetadata.HEADER_INJECTION.reporter(), sarif); - this.remediator = HeaderInjectionRemediator.DEFAULT; + this.remediator = new HeaderInjectionRemediator<>(); } @Override @@ -43,7 +46,9 @@ public CodemodFileScanningResult visit( 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()); + r -> Optional.of(r.getLocations().get(0).getPhysicalLocation().getRegion().getEndLine()), + r -> + Optional.of( + r.getLocations().get(0).getPhysicalLocation().getRegion().getStartColumn())); } } diff --git a/core-codemods/src/main/java/io/codemodder/codemods/codeql/CodeQLSQLInjectionCodemod.java b/core-codemods/src/main/java/io/codemodder/codemods/codeql/CodeQLSQLInjectionCodemod.java index 7e1eeb064..fddff61bb 100644 --- a/core-codemods/src/main/java/io/codemodder/codemods/codeql/CodeQLSQLInjectionCodemod.java +++ b/core-codemods/src/main/java/io/codemodder/codemods/codeql/CodeQLSQLInjectionCodemod.java @@ -2,12 +2,16 @@ import com.contrastsecurity.sarif.Result; import com.github.javaparser.ast.CompilationUnit; +import com.github.javaparser.ast.expr.Expression; import io.codemodder.*; +import io.codemodder.ast.ASTs; import io.codemodder.codetf.DetectorRule; import io.codemodder.providers.sarif.codeql.ProvidedCodeQLScan; +import io.codemodder.remediation.FixCandidateSearcher; import io.codemodder.remediation.GenericRemediationMetadata; import io.codemodder.remediation.Remediator; -import io.codemodder.remediation.sqlinjection.SQLInjectionRemediator; +import io.codemodder.remediation.SearcherStrategyRemediator; +import io.codemodder.remediation.sqlinjection.SQLInjectionFixComposer; import java.util.Optional; import javax.inject.Inject; @@ -25,7 +29,26 @@ public final class CodeQLSQLInjectionCodemod extends CodeQLRemediationCodemod { public CodeQLSQLInjectionCodemod( @ProvidedCodeQLScan(ruleId = "java/sql-injection") final RuleSarif sarif) { super(GenericRemediationMetadata.SQL_INJECTION.reporter(), sarif); - this.remediator = new SQLInjectionRemediator<>(); + this.remediator = + new SearcherStrategyRemediator.Builder() + .withSearcherStrategyPair( + new FixCandidateSearcher.Builder() + .withMatcher( + n -> + Optional.empty() + // is the argument of the call + .or( + () -> + Optional.of(n) + .map( + m -> + m instanceof Expression ? (Expression) m : null) + .flatMap(ASTs::isArgumentOfMethodCall) + .filter(SQLInjectionFixComposer::match)) + .isPresent()) + .build(), + new SQLInjectionFixComposer()) + .build(); } @Override diff --git a/core-codemods/src/main/java/io/codemodder/codemods/codeql/CodeQLSSRFCodemod.java b/core-codemods/src/main/java/io/codemodder/codemods/codeql/CodeQLSSRFCodemod.java index 6eb6c4801..ceddb7926 100644 --- a/core-codemods/src/main/java/io/codemodder/codemods/codeql/CodeQLSSRFCodemod.java +++ b/core-codemods/src/main/java/io/codemodder/codemods/codeql/CodeQLSSRFCodemod.java @@ -1,11 +1,14 @@ package io.codemodder.codemods.codeql; +import com.contrastsecurity.sarif.Result; 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.Remediator; import io.codemodder.remediation.ssrf.SSRFRemediator; +import java.util.Optional; import javax.inject.Inject; /** A codemod for automatically fixing SQL injection from CodeQL. */ @@ -16,12 +19,12 @@ executionPriority = CodemodExecutionPriority.HIGH) public final class CodeQLSSRFCodemod extends CodeQLRemediationCodemod { - private final SSRFRemediator remediator; + private final Remediator remediator; @Inject public CodeQLSSRFCodemod(@ProvidedCodeQLScan(ruleId = "java/ssrf") final RuleSarif sarif) { super(GenericRemediationMetadata.SSRF.reporter(), sarif); - this.remediator = SSRFRemediator.DEFAULT; + this.remediator = new SSRFRemediator<>(); } @Override @@ -42,7 +45,9 @@ public CodemodFileScanningResult visit( 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()); + r -> Optional.of(r.getLocations().get(0).getPhysicalLocation().getRegion().getEndLine()), + r -> + Optional.of( + r.getLocations().get(0).getPhysicalLocation().getRegion().getStartColumn())); } } diff --git a/core-codemods/src/main/java/io/codemodder/codemods/semgrep/SemgrepJavaDeserializationCodemod.java b/core-codemods/src/main/java/io/codemodder/codemods/semgrep/SemgrepJavaDeserializationCodemod.java index 39c54de43..6446d968c 100644 --- a/core-codemods/src/main/java/io/codemodder/codemods/semgrep/SemgrepJavaDeserializationCodemod.java +++ b/core-codemods/src/main/java/io/codemodder/codemods/semgrep/SemgrepJavaDeserializationCodemod.java @@ -1,5 +1,6 @@ package io.codemodder.codemods.semgrep; +import com.contrastsecurity.sarif.Result; import com.github.javaparser.ast.CompilationUnit; import io.codemodder.Codemod; import io.codemodder.CodemodExecutionPriority; @@ -12,7 +13,9 @@ import io.codemodder.codetf.DetectorRule; import io.codemodder.providers.sarif.semgrep.ProvidedSemgrepScan; import io.codemodder.remediation.GenericRemediationMetadata; +import io.codemodder.remediation.Remediator; import io.codemodder.remediation.javadeserialization.JavaDeserializationRemediator; +import java.util.Optional; import javax.inject.Inject; /** @@ -26,7 +29,7 @@ importance = Importance.HIGH) public final class SemgrepJavaDeserializationCodemod extends SemgrepJavaParserChanger { - private final JavaDeserializationRemediator remediator; + private final Remediator remediator; @Inject public SemgrepJavaDeserializationCodemod( @@ -34,7 +37,7 @@ public SemgrepJavaDeserializationCodemod( ruleId = "java.lang.security.audit.object-deserialization.object-deserialization") final RuleSarif sarif) { super(GenericRemediationMetadata.DESERIALIZATION.reporter(), sarif); - this.remediator = JavaDeserializationRemediator.DEFAULT; + this.remediator = new JavaDeserializationRemediator<>(); } @Override @@ -55,7 +58,11 @@ public CodemodFileScanningResult visit( 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()); + r -> + Optional.ofNullable( + r.getLocations().get(0).getPhysicalLocation().getRegion().getEndLine()), + r -> + Optional.ofNullable( + r.getLocations().get(0).getPhysicalLocation().getRegion().getStartColumn())); } } diff --git a/core-codemods/src/main/java/io/codemodder/codemods/semgrep/SemgrepMissingSecureFlagCodemod.java b/core-codemods/src/main/java/io/codemodder/codemods/semgrep/SemgrepMissingSecureFlagCodemod.java index 50b13ea2d..83a195b19 100644 --- a/core-codemods/src/main/java/io/codemodder/codemods/semgrep/SemgrepMissingSecureFlagCodemod.java +++ b/core-codemods/src/main/java/io/codemodder/codemods/semgrep/SemgrepMissingSecureFlagCodemod.java @@ -1,5 +1,6 @@ package io.codemodder.codemods.semgrep; +import com.contrastsecurity.sarif.Result; import com.github.javaparser.ast.CompilationUnit; import io.codemodder.Codemod; import io.codemodder.CodemodExecutionPriority; @@ -12,7 +13,9 @@ import io.codemodder.codetf.DetectorRule; import io.codemodder.providers.sarif.semgrep.ProvidedSemgrepScan; import io.codemodder.remediation.GenericRemediationMetadata; +import io.codemodder.remediation.Remediator; import io.codemodder.remediation.missingsecureflag.MissingSecureFlagRemediator; +import java.util.Optional; import javax.inject.Inject; /** @@ -27,7 +30,7 @@ importance = Importance.HIGH) public final class SemgrepMissingSecureFlagCodemod extends SemgrepJavaParserChanger { - private final MissingSecureFlagRemediator remediator; + private final Remediator remediator; @Inject public SemgrepMissingSecureFlagCodemod( @@ -36,7 +39,7 @@ public SemgrepMissingSecureFlagCodemod( "java.lang.security.audit.cookie-missing-secure-flag.cookie-missing-secure-flag") final RuleSarif sarif) { super(GenericRemediationMetadata.MISSING_SECURE_FLAG.reporter(), sarif); - this.remediator = MissingSecureFlagRemediator.DEFAULT; + this.remediator = new MissingSecureFlagRemediator<>(); } @Override @@ -57,7 +60,11 @@ public CodemodFileScanningResult visit( 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()); + r -> + Optional.ofNullable( + r.getLocations().get(0).getPhysicalLocation().getRegion().getEndLine()), + r -> + Optional.ofNullable( + r.getLocations().get(0).getPhysicalLocation().getRegion().getStartColumn())); } } diff --git a/core-codemods/src/main/java/io/codemodder/codemods/semgrep/SemgrepReflectionInjectionCodemod.java b/core-codemods/src/main/java/io/codemodder/codemods/semgrep/SemgrepReflectionInjectionCodemod.java index abff5b327..a1dfa1acc 100644 --- a/core-codemods/src/main/java/io/codemodder/codemods/semgrep/SemgrepReflectionInjectionCodemod.java +++ b/core-codemods/src/main/java/io/codemodder/codemods/semgrep/SemgrepReflectionInjectionCodemod.java @@ -1,5 +1,6 @@ package io.codemodder.codemods.semgrep; +import com.contrastsecurity.sarif.Result; import com.github.javaparser.ast.CompilationUnit; import io.codemodder.Codemod; import io.codemodder.CodemodExecutionPriority; @@ -12,7 +13,9 @@ import io.codemodder.codetf.DetectorRule; import io.codemodder.providers.sarif.semgrep.ProvidedSemgrepScan; import io.codemodder.remediation.GenericRemediationMetadata; +import io.codemodder.remediation.Remediator; import io.codemodder.remediation.reflectioninjection.ReflectionInjectionRemediator; +import java.util.Optional; import javax.inject.Inject; /** @@ -26,14 +29,14 @@ importance = Importance.MEDIUM) public final class SemgrepReflectionInjectionCodemod extends SemgrepJavaParserChanger { - private final ReflectionInjectionRemediator remediator; + private final Remediator remediator; @Inject public SemgrepReflectionInjectionCodemod( @ProvidedSemgrepScan(ruleId = "java.lang.security.audit.unsafe-reflection.unsafe-reflection") final RuleSarif sarif) { super(GenericRemediationMetadata.REFLECTION_INJECTION.reporter(), sarif); - this.remediator = ReflectionInjectionRemediator.DEFAULT; + this.remediator = new ReflectionInjectionRemediator<>(); } @Override @@ -54,7 +57,11 @@ public CodemodFileScanningResult visit( 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()); + r -> + Optional.ofNullable( + r.getLocations().get(0).getPhysicalLocation().getRegion().getEndLine()), + r -> + Optional.ofNullable( + r.getLocations().get(0).getPhysicalLocation().getRegion().getStartColumn())); } } diff --git a/core-codemods/src/main/java/io/codemodder/codemods/semgrep/SemgrepSQLInjectionCodemod.java b/core-codemods/src/main/java/io/codemodder/codemods/semgrep/SemgrepSQLInjectionCodemod.java index a2a3ae50b..53952d44c 100644 --- a/core-codemods/src/main/java/io/codemodder/codemods/semgrep/SemgrepSQLInjectionCodemod.java +++ b/core-codemods/src/main/java/io/codemodder/codemods/semgrep/SemgrepSQLInjectionCodemod.java @@ -12,9 +12,11 @@ import io.codemodder.SarifFindingKeyUtil; import io.codemodder.codetf.DetectorRule; import io.codemodder.providers.sarif.semgrep.ProvidedSemgrepScan; +import io.codemodder.remediation.FixCandidateSearcher; import io.codemodder.remediation.GenericRemediationMetadata; import io.codemodder.remediation.Remediator; -import io.codemodder.remediation.sqlinjection.SQLInjectionRemediator; +import io.codemodder.remediation.SearcherStrategyRemediator; +import io.codemodder.remediation.sqlinjection.SQLInjectionFixComposer; import java.util.Optional; import javax.inject.Inject; @@ -36,7 +38,19 @@ public SemgrepSQLInjectionCodemod( @ProvidedSemgrepScan(ruleId = "java.lang.security.audit.sqli.jdbc-sqli.jdbc-sqli") final RuleSarif sarif) { super(GenericRemediationMetadata.SQL_INJECTION.reporter(), sarif); - this.remediator = new SQLInjectionRemediator<>(); + this.remediator = + new SearcherStrategyRemediator.Builder() + .withSearcherStrategyPair( + new FixCandidateSearcher.Builder() + .withMatcher( + n -> + Optional.empty() + // is the call itself + .or(() -> Optional.of(n).filter(SQLInjectionFixComposer::match)) + .isPresent()) + .build(), + new SQLInjectionFixComposer()) + .build(); } @Override diff --git a/core-codemods/src/main/java/io/codemodder/codemods/semgrep/SemgrepSSRFCodemod.java b/core-codemods/src/main/java/io/codemodder/codemods/semgrep/SemgrepSSRFCodemod.java index 851b65651..e3bc7b2da 100644 --- a/core-codemods/src/main/java/io/codemodder/codemods/semgrep/SemgrepSSRFCodemod.java +++ b/core-codemods/src/main/java/io/codemodder/codemods/semgrep/SemgrepSSRFCodemod.java @@ -1,5 +1,6 @@ package io.codemodder.codemods.semgrep; +import com.contrastsecurity.sarif.Result; import com.github.javaparser.ast.CompilationUnit; import io.codemodder.Codemod; import io.codemodder.CodemodExecutionPriority; @@ -12,7 +13,9 @@ import io.codemodder.codetf.DetectorRule; import io.codemodder.providers.sarif.semgrep.ProvidedSemgrepScan; import io.codemodder.remediation.GenericRemediationMetadata; +import io.codemodder.remediation.Remediator; import io.codemodder.remediation.ssrf.SSRFRemediator; +import java.util.Optional; import javax.inject.Inject; /** @@ -26,7 +29,7 @@ importance = Importance.HIGH) public final class SemgrepSSRFCodemod extends SemgrepJavaParserChanger { - private final SSRFRemediator remediator; + private final Remediator remediator; @Inject public SemgrepSSRFCodemod( @@ -34,7 +37,7 @@ public SemgrepSSRFCodemod( ruleId = "java.spring.security.injection.tainted-url-host.tainted-url-host") final RuleSarif sarif) { super(GenericRemediationMetadata.SSRF.reporter(), sarif); - this.remediator = SSRFRemediator.DEFAULT; + this.remediator = new SSRFRemediator<>(); } @Override @@ -55,7 +58,11 @@ public CodemodFileScanningResult visit( 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()); + r -> + Optional.ofNullable( + r.getLocations().get(0).getPhysicalLocation().getRegion().getEndLine()), + r -> + Optional.ofNullable( + r.getLocations().get(0).getPhysicalLocation().getRegion().getStartColumn())); } } diff --git a/framework/codemodder-base/src/main/java/io/codemodder/Either.java b/framework/codemodder-base/src/main/java/io/codemodder/Either.java index ee439f469..00e8db116 100644 --- a/framework/codemodder-base/src/main/java/io/codemodder/Either.java +++ b/framework/codemodder-base/src/main/java/io/codemodder/Either.java @@ -1,5 +1,6 @@ package io.codemodder; +import java.util.Objects; import java.util.Optional; import java.util.function.Consumer; import java.util.function.Function; @@ -38,48 +39,48 @@ public R getRight() { /** Returns an {@link Either} containing {@code value} as a left value. */ public static Either left(A value) { - return new Either<>(value, null); + return new Either<>(Objects.requireNonNull(value), null); } /** Returns an {@link Either} containing {@code value} as a right value. */ public static Either right(B value) { - return new Either<>(null, value); + return new Either<>(null, Objects.requireNonNull(value)); } /** Applies {@code func} to the left value if present. */ public Either map(Function func) { - if (this.isLeft()) return new Either<>(func.apply(this.leftValue), null); - else return new Either<>(null, this.rightValue); + if (this.isLeft()) return Either.left(func.apply(this.leftValue)); + else return Either.right(this.rightValue); } /** * Applies {@code func} to the right value if present and returns an {@code Either} containing the - * result. Otherwise returns an {@code Either} containing the left value. + * result. Otherwise, returns an {@code Either} containing the left value. */ public Either flatMap(Function> func) { if (this.isLeft()) { var other = func.apply(this.leftValue); - if (other.isLeft()) return new Either<>(other.leftValue, null); - else return new Either<>(null, other.rightValue); - } else return new Either<>(null, this.rightValue); + if (other.isLeft()) return Either.left(other.leftValue); + else return Either.right(other.rightValue); + } else return Either.right(this.rightValue); } /** Applies {@code func} to the left value if present. */ public Either mapRight(Function func) { - if (this.isRight()) return new Either<>(null, func.apply(this.rightValue)); - else return new Either<>(this.leftValue, null); + if (this.isRight()) return Either.right(func.apply(this.rightValue)); + else return Either.left(this.leftValue); } /** * Applies {@code func} to the right value if present and returns an {@code Either} containing the - * result. Otherwise returns an {@code Either} containing the left value. + * result. Otherwise, returns an {@code Either} containing the left value. */ public Either flatMapRight(Function> func) { if (this.isRight()) { var other = func.apply(this.rightValue); - if (other.isRight()) return new Either<>(null, other.rightValue); - else return new Either<>(other.leftValue, null); - } else return new Either<>(this.leftValue, null); + if (other.isRight()) return Either.right(other.rightValue); + else return Either.left(other.leftValue); + } else return Either.left(this.leftValue); } /** diff --git a/framework/codemodder-base/src/main/java/io/codemodder/remediation/DefaultFixCandidateSearcher.java b/framework/codemodder-base/src/main/java/io/codemodder/remediation/DefaultFixCandidateSearcher.java index 730454ea6..b46a44b36 100644 --- a/framework/codemodder-base/src/main/java/io/codemodder/remediation/DefaultFixCandidateSearcher.java +++ b/framework/codemodder-base/src/main/java/io/codemodder/remediation/DefaultFixCandidateSearcher.java @@ -59,15 +59,18 @@ public FixCandidateSearchResults search( .map(issueEndLine -> matches(issueStartLine, nodeStartLine, issueEndLine)) .orElse(matches(issueStartLine, nodeStartLine)); }) - // if column info is present, check if the column is contained in the node's range + // if column info is present, check if the node starts after the issue start + // coordinates .filter( n -> maybeColumn .map( column -> n.getRange() - .orElseThrow() - .contains(new Position(issueStartLine, column))) + .orElseThrow() + .begin + .compareTo(new Position(issueStartLine, column)) + >= 0) .orElse(true)) .toList(); if (nodesForIssue.isEmpty()) { @@ -107,8 +110,7 @@ public List> fixCandidates() { static boolean matches( final int issueStartLine, final int startNodeLine, final int issueEndLine) { // if the issue spans multiple lines, the node must be within that range - return matches(issueStartLine, startNodeLine) - && isInBetween(startNodeLine, issueStartLine, issueEndLine); + return isInBetween(startNodeLine, issueStartLine, issueEndLine); } static boolean matches(final int issueStartLine, final int startNodeLine) { diff --git a/framework/codemodder-base/src/main/java/io/codemodder/remediation/headerinjection/DefaultHeaderInjectionRemediator.java b/framework/codemodder-base/src/main/java/io/codemodder/remediation/headerinjection/DefaultHeaderInjectionRemediator.java deleted file mode 100644 index e2c89cf3d..000000000 --- a/framework/codemodder-base/src/main/java/io/codemodder/remediation/headerinjection/DefaultHeaderInjectionRemediator.java +++ /dev/null @@ -1,120 +0,0 @@ -package io.codemodder.remediation.headerinjection; - -import static io.codemodder.javaparser.JavaParserTransformer.wrap; - -import com.github.javaparser.StaticJavaParser; -import com.github.javaparser.ast.CompilationUnit; -import com.github.javaparser.ast.NodeList; -import com.github.javaparser.ast.body.ClassOrInterfaceDeclaration; -import com.github.javaparser.ast.body.MethodDeclaration; -import com.github.javaparser.ast.expr.Expression; -import com.github.javaparser.ast.expr.MethodCallExpr; -import com.github.javaparser.ast.expr.StringLiteralExpr; -import io.codemodder.CodemodChange; -import io.codemodder.CodemodFileScanningResult; -import io.codemodder.codetf.DetectorRule; -import io.codemodder.codetf.FixedFinding; -import io.codemodder.remediation.LegacyFixCandidate; -import io.codemodder.remediation.LegacyFixCandidateSearchResults; -import io.codemodder.remediation.LegacyFixCandidateSearcher; -import io.codemodder.remediation.MethodOrConstructor; -import java.util.ArrayList; -import java.util.List; -import java.util.Set; -import java.util.function.Function; - -final class DefaultHeaderInjectionRemediator implements HeaderInjectionRemediator { - - private static final Set setHeaderNames = Set.of("setHeader", "addHeader"); - - private static final String validatorMethodName = "stripNewlines"; - private final String fixMethodCode; - - DefaultHeaderInjectionRemediator() { - this.fixMethodCode = - """ - private static String stripNewlines(final String s) { - return s.replaceAll("[\\n\\r]", ""); - } - """; - } - - @Override - public CodemodFileScanningResult remediateAll( - final CompilationUnit cu, - final String path, - final DetectorRule detectorRule, - final List issuesForFile, - final Function getKey, - final Function getStartLine, - final Function getEndLine, - final Function getStartColumn) { - - LegacyFixCandidateSearcher searcher = - new LegacyFixCandidateSearcher.Builder() - .withMatcher(mce -> mce.isMethodCallWithNameIn(setHeaderNames)) - .withMatcher(MethodOrConstructor::isMethodCallWithScope) - .withMatcher(mce -> mce.getArguments().size() == 2) - .withMatcher(mce -> !(mce.getArguments().get(1) instanceof StringLiteralExpr)) - .build(); - - LegacyFixCandidateSearchResults results = - searcher.search( - cu, - path, - detectorRule, - issuesForFile, - getKey, - getStartLine, - getEndLine, - getStartColumn); - - List changes = new ArrayList<>(); - for (LegacyFixCandidate legacyFixCandidate : results.fixCandidates()) { - List issues = legacyFixCandidate.issues(); - - MethodCallExpr setHeaderCall = legacyFixCandidate.call().asMethodCall(); - Expression headerValueArgument = setHeaderCall.getArgument(1); - wrap(headerValueArgument).withScopelessMethod(validatorMethodName); - - // add the validation method if it's not already present - ClassOrInterfaceDeclaration parentClass = - setHeaderCall.findAncestor(ClassOrInterfaceDeclaration.class).get(); - if (parentClass.isInterface()) { - MethodCallExpr inlinedStripCall = - new MethodCallExpr( - headerValueArgument, - "replaceAll", - NodeList.nodeList(new StringLiteralExpr("[\\n\\r]"), new StringLiteralExpr(""))); - setHeaderCall.getArguments().set(1, inlinedStripCall); - } else { - boolean alreadyHasResourceValidationCallPresent = - parentClass.findAll(MethodDeclaration.class).stream() - .anyMatch( - md -> - md.getNameAsString().equals(validatorMethodName) - && md.getParameters().size() == 1 - && md.getParameters().get(0).getTypeAsString().equals("String")); - - if (!alreadyHasResourceValidationCallPresent) { - // one might be tempted to cache this result, but then it will be a shared resource with - // shared CST metadata and cause bugs - MethodDeclaration fixMethod = StaticJavaParser.parseMethodDeclaration(fixMethodCode); - - // add the method to the class - parentClass.addMember(fixMethod); - } - } - - // all the line numbers should be the same, so we just grab the first one - int line = getStartLine.apply(legacyFixCandidate.issues().get(0)); - List fixedFindings = - issues.stream() - .map(issue -> new FixedFinding(getKey.apply(issue), detectorRule)) - .toList(); - changes.add(CodemodChange.from(line, List.of(), fixedFindings)); - } - - return CodemodFileScanningResult.from(changes, results.unfixableFindings()); - } -} diff --git a/framework/codemodder-base/src/main/java/io/codemodder/remediation/headerinjection/HeaderInjectionFixStrategy.java b/framework/codemodder-base/src/main/java/io/codemodder/remediation/headerinjection/HeaderInjectionFixStrategy.java new file mode 100644 index 000000000..52c45d441 --- /dev/null +++ b/framework/codemodder-base/src/main/java/io/codemodder/remediation/headerinjection/HeaderInjectionFixStrategy.java @@ -0,0 +1,75 @@ +package io.codemodder.remediation.headerinjection; + +import static io.codemodder.javaparser.JavaParserTransformer.wrap; + +import com.github.javaparser.StaticJavaParser; +import com.github.javaparser.ast.CompilationUnit; +import com.github.javaparser.ast.Node; +import com.github.javaparser.ast.NodeList; +import com.github.javaparser.ast.body.ClassOrInterfaceDeclaration; +import com.github.javaparser.ast.body.MethodDeclaration; +import com.github.javaparser.ast.expr.Expression; +import com.github.javaparser.ast.expr.MethodCallExpr; +import com.github.javaparser.ast.expr.StringLiteralExpr; +import io.codemodder.remediation.*; +import java.util.Optional; + +/** Default strategy to harden header injection vulnerabilities. */ +public final class HeaderInjectionFixStrategy implements RemediationStrategy { + + private static final String validatorMethodName = "stripNewlines"; + + private static final String fixMethodCode = + """ + private static String stripNewlines(final String s) { + return s.replaceAll("[\\n\\r]", ""); + } + """; + + @Override + public SuccessOrReason fix(final CompilationUnit cu, final Node node) { + var maybeMethodCall = + Optional.of(node).map(n -> n instanceof MethodCallExpr ? (MethodCallExpr) n : null); + if (maybeMethodCall.isEmpty()) { + return SuccessOrReason.reason("Not a method call"); + } + MethodCallExpr setHeaderCall = maybeMethodCall.get(); + Expression headerValueArgument = setHeaderCall.getArgument(1); + wrap(headerValueArgument).withScopelessMethod(validatorMethodName); + + Optional maybeParentClass = + setHeaderCall.findAncestor(ClassOrInterfaceDeclaration.class); + if (maybeParentClass.isEmpty()) { + return SuccessOrReason.reason("Could not find parent class"); + } + var parentClass = maybeParentClass.get(); + + // add the validation method if it's not already present + if (parentClass.isInterface()) { + MethodCallExpr inlinedStripCall = + new MethodCallExpr( + headerValueArgument, + "replaceAll", + NodeList.nodeList(new StringLiteralExpr("[\\n\\r]"), new StringLiteralExpr(""))); + setHeaderCall.getArguments().set(1, inlinedStripCall); + } else { + boolean alreadyHasResourceValidationCallPresent = + parentClass.findAll(MethodDeclaration.class).stream() + .anyMatch( + md -> + md.getNameAsString().equals(validatorMethodName) + && md.getParameters().size() == 1 + && md.getParameters().get(0).getTypeAsString().equals("String")); + + if (!alreadyHasResourceValidationCallPresent) { + // one might be tempted to cache this result, but then it will be a shared resource with + // shared CST metadata and cause bugs + MethodDeclaration fixMethod = StaticJavaParser.parseMethodDeclaration(fixMethodCode); + + // add the method to the class + parentClass.addMember(fixMethod); + } + } + return SuccessOrReason.success(); + } +} diff --git a/framework/codemodder-base/src/main/java/io/codemodder/remediation/headerinjection/HeaderInjectionRemediator.java b/framework/codemodder-base/src/main/java/io/codemodder/remediation/headerinjection/HeaderInjectionRemediator.java index 636176b79..5d0b6d8f4 100644 --- a/framework/codemodder-base/src/main/java/io/codemodder/remediation/headerinjection/HeaderInjectionRemediator.java +++ b/framework/codemodder-base/src/main/java/io/codemodder/remediation/headerinjection/HeaderInjectionRemediator.java @@ -1,25 +1,63 @@ package io.codemodder.remediation.headerinjection; import com.github.javaparser.ast.CompilationUnit; +import com.github.javaparser.ast.Node; +import com.github.javaparser.ast.expr.MethodCallExpr; +import com.github.javaparser.ast.expr.StringLiteralExpr; import io.codemodder.CodemodFileScanningResult; import io.codemodder.codetf.DetectorRule; -import java.util.List; +import io.codemodder.remediation.*; +import java.util.*; import java.util.function.Function; -/** Remediates header injection vulnerabilities. */ -public interface HeaderInjectionRemediator { +/** + * Fixes header injection pointed by issues. + * + * @param + */ +public final class HeaderInjectionRemediator implements Remediator { - /** Remediate all header injection vulnerabilities in the given compilation unit. */ - CodemodFileScanningResult remediateAll( + private final SearcherStrategyRemediator searchStrategyRemediator; + + private static final Set setHeaderNames = Set.of("setHeader", "addHeader"); + + public HeaderInjectionRemediator() { + this.searchStrategyRemediator = + new SearcherStrategyRemediator.Builder() + .withSearcherStrategyPair( + new FixCandidateSearcher.Builder() + .withMatcher( + node -> + Optional.of(node) + .map(n -> n instanceof MethodCallExpr ? (MethodCallExpr) n : null) + .filter(Node::hasScope) + .filter(mce -> setHeaderNames.contains(mce.getNameAsString())) + .filter(mce -> mce.getArguments().size() == 2) + .filter(mce -> !(mce.getArgument(1) instanceof StringLiteralExpr)) + .isPresent()) + .build(), + new HeaderInjectionFixStrategy()) + .build(); + } + + @Override + public CodemodFileScanningResult remediateAll( CompilationUnit cu, String path, DetectorRule detectorRule, - List issuesForFile, - Function getKey, - Function getStartLine, - Function getEndLine, - Function getStartColumn); - - /** The default header injection remediation strategy. */ - HeaderInjectionRemediator DEFAULT = new DefaultHeaderInjectionRemediator(); + Collection findingsForPath, + Function findingIdExtractor, + Function findingStartLineExtractor, + Function> findingEndLineExtractor, + Function> findingColumnExtractor) { + return searchStrategyRemediator.remediateAll( + cu, + path, + detectorRule, + findingsForPath, + findingIdExtractor, + findingStartLineExtractor, + findingEndLineExtractor, + findingColumnExtractor); + } } diff --git a/framework/codemodder-base/src/main/java/io/codemodder/remediation/javadeserialization/DefaultJavaDeserializationRemediator.java b/framework/codemodder-base/src/main/java/io/codemodder/remediation/javadeserialization/DefaultJavaDeserializationRemediator.java deleted file mode 100644 index 5aadb13da..000000000 --- a/framework/codemodder-base/src/main/java/io/codemodder/remediation/javadeserialization/DefaultJavaDeserializationRemediator.java +++ /dev/null @@ -1,194 +0,0 @@ -package io.codemodder.remediation.javadeserialization; - -import static io.codemodder.javaparser.JavaParserTransformer.replace; - -import com.github.javaparser.ast.CompilationUnit; -import com.github.javaparser.ast.Node; -import com.github.javaparser.ast.body.VariableDeclarator; -import com.github.javaparser.ast.expr.Expression; -import com.github.javaparser.ast.expr.MethodCallExpr; -import com.github.javaparser.ast.expr.ObjectCreationExpr; -import io.codemodder.CodemodChange; -import io.codemodder.CodemodFileScanningResult; -import io.codemodder.DependencyGAV; -import io.codemodder.ast.ASTs; -import io.codemodder.ast.LocalDeclaration; -import io.codemodder.codetf.DetectorRule; -import io.codemodder.codetf.FixedFinding; -import io.codemodder.codetf.UnfixedFinding; -import io.codemodder.remediation.LegacyFixCandidate; -import io.codemodder.remediation.LegacyFixCandidateSearchResults; -import io.codemodder.remediation.LegacyFixCandidateSearcher; -import io.codemodder.remediation.MethodOrConstructor; -import io.github.pixee.security.ObjectInputFilters; -import java.util.ArrayList; -import java.util.List; -import java.util.Optional; -import java.util.function.Function; -import org.jetbrains.annotations.NotNull; - -final class DefaultJavaDeserializationRemediator implements JavaDeserializationRemediator { - - @Override - public CodemodFileScanningResult remediateAll( - final CompilationUnit cu, - final String path, - final DetectorRule detectorRule, - final List issuesForFile, - final Function getKey, - final Function getStartLine, - final Function getEndLine, - final Function getStartColumn) { - - LegacyFixCandidateSearcher searcher = - new LegacyFixCandidateSearcher.Builder() - .withMethodName("readObject") - .withMatcher(MethodOrConstructor::isMethodCallWithScope) - .withMatcher(mce -> mce.getArguments().isEmpty()) - .build(); - - // search for readObject() calls on those lines, assuming the tool points there - LegacyFixCandidateSearchResults results = - searcher.search( - cu, - path, - detectorRule, - issuesForFile, - getKey, - getStartLine, - getEndLine, - getStartColumn); - - if (results.fixCandidates().isEmpty()) { - // try searching for matching ObjectInputStream creation objects, maybe that's where they're - // pointing - searcher = - new LegacyFixCandidateSearcher.Builder() - .withMatcher(mc -> mc.isConstructorForType("ObjectInputStream")) - .build(); - - results = - searcher.search( - cu, - path, - detectorRule, - issuesForFile, - getKey, - getStartLine, - getEndLine, - getStartColumn); - } - - List changes = new ArrayList<>(); - List unfixedFindings = new ArrayList<>(); - - for (LegacyFixCandidate legacyFixCandidate : results.fixCandidates()) { - List issues = legacyFixCandidate.issues(); - MethodOrConstructor call = legacyFixCandidate.call(); - - if (call.isConstructor()) { - // we're pointing to the readObject(), fix and move on - fixObjectInputStreamCreation((ObjectCreationExpr) call.asNode()); - CodemodChange change = buildFixChange(detectorRule, getKey, getStartLine, issues); - changes.add(change); - continue; - } - - // we're pointing to the readObject(), so we must work backwards to find the declaration of - // the ObjectInputStream - MethodCallExpr mce = (MethodCallExpr) call.asNode(); - Expression callScope = mce.getScope().get(); - if (!callScope.isNameExpr()) { - // can't fix these - issues.stream() - .map( - i -> - new UnfixedFinding( - getKey.apply(i), - detectorRule, - path, - getStartLine.apply(i), - "Unexpected shape")) - .forEach(unfixedFindings::add); - continue; - } - - Optional declaration = - ASTs.findEarliestLocalDeclarationOf(callScope.asNameExpr().getName()); - if (declaration.isEmpty()) { - issues.stream() - .map( - i -> - new UnfixedFinding( - getKey.apply(i), - detectorRule, - path, - getStartLine.apply(i), - "No declaration found")) - .forEach(unfixedFindings::add); - continue; - } - - LocalDeclaration localDeclaration = declaration.get(); - Node varDeclarationAndExpr = localDeclaration.getDeclaration(); - if (varDeclarationAndExpr instanceof VariableDeclarator varDec) { - Optional initializer = varDec.getInitializer(); - if (initializer.isEmpty()) { - issues.stream() - .map( - i -> - new UnfixedFinding( - getKey.apply(i), - detectorRule, - path, - getStartLine.apply(i), - "No initializer found")) - .forEach(unfixedFindings::add); - continue; - } - - Expression expression = initializer.get(); - if (expression instanceof ObjectCreationExpr objCreation) { - fixObjectInputStreamCreation(objCreation); - CodemodChange change = buildFixChange(detectorRule, getKey, getStartLine, issues); - changes.add(change); - } - } else { - issues.stream() - .map( - i -> - new UnfixedFinding( - getKey.apply(i), - detectorRule, - path, - getStartLine.apply(i), - "Unexpected declaration type")) - .forEach(unfixedFindings::add); - } - } - - unfixedFindings.addAll(results.unfixableFindings()); - return CodemodFileScanningResult.from(changes, unfixedFindings); - } - - /** - * Build a {@link io.codemodder.CodemodChange} for this code change that fixes the given issues. - */ - private static @NotNull CodemodChange buildFixChange( - final DetectorRule detectorRule, - final Function getKey, - final Function getLine, - final List issues) { - return CodemodChange.from( - getLine.apply(issues.get(0)), - List.of(DependencyGAV.JAVA_SECURITY_TOOLKIT), - issues.stream().map(i -> new FixedFinding(getKey.apply(i), detectorRule)).toList()); - } - - private void fixObjectInputStreamCreation(final ObjectCreationExpr objCreation) { - replace(objCreation) - .withStaticMethod(ObjectInputFilters.class.getName(), "createSafeObjectInputStream") - .withStaticImport() - .withSameArguments(); - } -} diff --git a/framework/codemodder-base/src/main/java/io/codemodder/remediation/javadeserialization/JavaDeserializationFixStrategy.java b/framework/codemodder-base/src/main/java/io/codemodder/remediation/javadeserialization/JavaDeserializationFixStrategy.java new file mode 100644 index 000000000..87f992ae5 --- /dev/null +++ b/framework/codemodder-base/src/main/java/io/codemodder/remediation/javadeserialization/JavaDeserializationFixStrategy.java @@ -0,0 +1,103 @@ +package io.codemodder.remediation.javadeserialization; + +import static io.codemodder.javaparser.JavaParserTransformer.replace; + +import com.github.javaparser.ast.CompilationUnit; +import com.github.javaparser.ast.Node; +import com.github.javaparser.ast.body.VariableDeclarator; +import com.github.javaparser.ast.expr.Expression; +import com.github.javaparser.ast.expr.MethodCallExpr; +import com.github.javaparser.ast.expr.NameExpr; +import com.github.javaparser.ast.expr.ObjectCreationExpr; +import io.codemodder.DependencyGAV; +import io.codemodder.Either; +import io.codemodder.ast.ASTs; +import io.codemodder.ast.LocalDeclaration; +import io.codemodder.remediation.*; +import io.github.pixee.security.ObjectInputFilters; +import java.util.List; +import java.util.Optional; + +/** Default strategy to hardens deserialization vulnerabilities. */ +public final class JavaDeserializationFixStrategy implements RemediationStrategy { + + /** + * Tries to find the construction of the scope of a call. + * + * @return + */ + private Either findConstructor(final MethodCallExpr call) { + + var maybeCallScope = call.getScope().map(s -> s instanceof NameExpr ? s.asNameExpr() : null); + if (maybeCallScope.isEmpty()) { + return Either.right("Unexpected shape"); + } + NameExpr callScope = maybeCallScope.get(); + + Optional declaration = + ASTs.findEarliestLocalDeclarationOf(callScope.getName()); + if (declaration.isEmpty()) { + return Either.right("No declaration found"); + } + + LocalDeclaration localDeclaration = declaration.get(); + Node varDeclarationAndExpr = localDeclaration.getDeclaration(); + if (varDeclarationAndExpr instanceof VariableDeclarator varDec) { + Optional initializer = varDec.getInitializer(); + if (initializer.isEmpty()) { + return Either.right("No initializer found"); + } + + Expression expression = initializer.get(); + if (expression instanceof ObjectCreationExpr objCreation) { + return Either.left(objCreation); + } + } else { + return Either.right("Unexpected declaration type"); + } + return Either.right("Failed to find constructor for associated call"); + } + + @Override + public SuccessOrReason fix(final CompilationUnit cu, final Node node) { + + Optional> maybeCallOrConstructor = + Optional.>empty() + .or( + () -> + node instanceof MethodCallExpr + ? Optional.of(Either.left((MethodCallExpr) node)) + : Optional.empty()) + .or( + () -> + node instanceof ObjectCreationExpr + ? Optional.of(Either.right((ObjectCreationExpr) node)) + : Optional.empty()); + if (maybeCallOrConstructor.isEmpty()) { + return SuccessOrReason.reason("Not a call or constructor"); + } + + Either maybeConstructor = + maybeCallOrConstructor + .get() + // If it is a call, we're pointing to the readObject(), so we must work backwards to + // find the + // declaration of the ObjectInputStream + .ifLeftOrElseGet(this::findConstructor, Either::left); + + // afailed to find the construction + if (maybeConstructor.isRight()) { + return SuccessOrReason.reason(maybeConstructor.getRight()); + } + + fixObjectInputStreamCreation(maybeConstructor.getLeft()); + return SuccessOrReason.success(List.of(DependencyGAV.JAVA_SECURITY_TOOLKIT)); + } + + private void fixObjectInputStreamCreation(final ObjectCreationExpr objCreation) { + replace(objCreation) + .withStaticMethod(ObjectInputFilters.class.getName(), "createSafeObjectInputStream") + .withStaticImport() + .withSameArguments(); + } +} diff --git a/framework/codemodder-base/src/main/java/io/codemodder/remediation/javadeserialization/JavaDeserializationRemediator.java b/framework/codemodder-base/src/main/java/io/codemodder/remediation/javadeserialization/JavaDeserializationRemediator.java index 216c76c5e..ce7320f99 100644 --- a/framework/codemodder-base/src/main/java/io/codemodder/remediation/javadeserialization/JavaDeserializationRemediator.java +++ b/framework/codemodder-base/src/main/java/io/codemodder/remediation/javadeserialization/JavaDeserializationRemediator.java @@ -1,25 +1,81 @@ package io.codemodder.remediation.javadeserialization; import com.github.javaparser.ast.CompilationUnit; +import com.github.javaparser.ast.Node; +import com.github.javaparser.ast.expr.MethodCallExpr; +import com.github.javaparser.ast.expr.ObjectCreationExpr; import io.codemodder.CodemodFileScanningResult; import io.codemodder.codetf.DetectorRule; -import java.util.List; +import io.codemodder.remediation.*; +import java.util.Collection; +import java.util.Optional; import java.util.function.Function; -/** Remediates Java deserialization vulnerabilities. */ -public interface JavaDeserializationRemediator { +/** + * Fixes issues of related to object deserialization + * + * @param + */ +public final class JavaDeserializationRemediator implements Remediator { - /** Remediate all Java deserialization vulnerabilities in the given compilation unit. */ - CodemodFileScanningResult remediateAll( + private final SearcherStrategyRemediator searchStrategyRemediator; + + public JavaDeserializationRemediator() { + this.searchStrategyRemediator = + new SearcherStrategyRemediator.Builder() + .withSearcherStrategyPair( + new FixCandidateSearcher.Builder() + .withMatcher( + node -> + Optional.empty() + .or( + () -> + Optional.of(node) + .map( + n -> + n instanceof MethodCallExpr + ? (MethodCallExpr) n + : null) + .filter(Node::hasScope) + .filter( + mce -> mce.getNameAsString().equals("readObject")) + .filter(mce -> mce.getArguments().isEmpty())) + .or( + () -> + Optional.of(node) + .map( + n -> + n instanceof ObjectCreationExpr + ? (ObjectCreationExpr) n + : null) + .filter( + oce -> + "ObjectInputStream" + .equals(oce.getTypeAsString()))) + .isPresent()) + .build(), + new JavaDeserializationFixStrategy()) + .build(); + } + + @Override + public CodemodFileScanningResult remediateAll( CompilationUnit cu, String path, DetectorRule detectorRule, - List issuesForFile, - Function getKey, - Function getStartLine, - Function getEndLine, - Function getColumn); - - /** The default header injection remediation strategy. */ - JavaDeserializationRemediator DEFAULT = new DefaultJavaDeserializationRemediator(); + Collection findingsForPath, + Function findingIdExtractor, + Function findingStartLineExtractor, + Function> findingEndLineExtractor, + Function> findingColumnExtractor) { + return searchStrategyRemediator.remediateAll( + cu, + path, + detectorRule, + findingsForPath, + findingIdExtractor, + findingStartLineExtractor, + findingEndLineExtractor, + findingColumnExtractor); + } } diff --git a/framework/codemodder-base/src/main/java/io/codemodder/remediation/missingsecureflag/DefaultMissingSecureFlagRemediator.java b/framework/codemodder-base/src/main/java/io/codemodder/remediation/missingsecureflag/DefaultMissingSecureFlagRemediator.java deleted file mode 100644 index ec8e1337b..000000000 --- a/framework/codemodder-base/src/main/java/io/codemodder/remediation/missingsecureflag/DefaultMissingSecureFlagRemediator.java +++ /dev/null @@ -1,126 +0,0 @@ -package io.codemodder.remediation.missingsecureflag; - -import com.github.javaparser.ast.CompilationUnit; -import com.github.javaparser.ast.NodeList; -import com.github.javaparser.ast.expr.BooleanLiteralExpr; -import com.github.javaparser.ast.expr.Expression; -import com.github.javaparser.ast.expr.MethodCallExpr; -import com.github.javaparser.ast.stmt.ExpressionStmt; -import com.github.javaparser.ast.stmt.Statement; -import io.codemodder.CodemodChange; -import io.codemodder.CodemodFileScanningResult; -import io.codemodder.ast.ASTTransforms; -import io.codemodder.codetf.DetectorRule; -import io.codemodder.codetf.FixedFinding; -import io.codemodder.codetf.UnfixedFinding; -import io.codemodder.remediation.LegacyFixCandidate; -import io.codemodder.remediation.LegacyFixCandidateSearchResults; -import io.codemodder.remediation.LegacyFixCandidateSearcher; -import io.codemodder.remediation.RemediationMessages; -import java.util.ArrayList; -import java.util.List; -import java.util.Optional; -import java.util.function.Function; -import org.jetbrains.annotations.NotNull; - -final class DefaultMissingSecureFlagRemediator implements MissingSecureFlagRemediator { - - @Override - public CodemodFileScanningResult remediateAll( - final CompilationUnit cu, - final String path, - final DetectorRule detectorRule, - final List issuesForFile, - final Function getKey, - final Function getStartLine, - final Function getEndLine, - final Function getStartColumn) { - - // this remediator assumes we're pointing to a response.addCookie() call - LegacyFixCandidateSearcher searcher = - new LegacyFixCandidateSearcher.Builder() - .withMethodName("addCookie") - .withMatcher(mce -> mce.getArguments().size() == 1) - .build(); - - LegacyFixCandidateSearchResults results = - searcher.search( - cu, - path, - detectorRule, - issuesForFile, - getKey, - getStartLine, - getEndLine, - getStartColumn); - - List changes = new ArrayList<>(); - List unfixedFindings = new ArrayList<>(); - - for (LegacyFixCandidate result : results.fixCandidates()) { - MethodCallExpr methodCallExpr = result.call().asMethodCall(); - List issues = result.issues(); - - if (methodCallExpr.getScope().isPresent()) { - - // This assumption is a bit strong, but covers the most common cases while avoiding weird - // ones - Optional maybeStmt = - methodCallExpr - .getParentNode() - .map(p -> p instanceof Statement ? (Statement) p : null) - .filter(Statement::isExpressionStmt); - - // We want to avoid things like: response.addCookie(new Cookie(...)), so we restrict - // ourselves - Optional maybeCookieExpression = - Optional.of(methodCallExpr.getArgument(0)) - .filter(expr -> expr.isNameExpr() || expr.isFieldAccessExpr()); - - if (maybeStmt.isPresent() && maybeCookieExpression.isPresent()) { - final var newStatement = - new ExpressionStmt( - new MethodCallExpr( - maybeCookieExpression.get(), - "setSecure", - new NodeList<>(new BooleanLiteralExpr(true)))); - - ASTTransforms.addStatementBeforeStatement(maybeStmt.get(), newStatement); - - FixedFinding fixedFinding = new FixedFinding(getKey.apply(issues.get(0)), detectorRule); - CodemodChange change = - CodemodChange.from( - result.call().getRange().begin.line, List.of(), List.of(fixedFinding)); - changes.add(change); - } else { - List unfixedFindingsToAdd = - createUnfixedFindingList(path, detectorRule, getKey, getStartLine, issues); - unfixedFindings.addAll(unfixedFindingsToAdd); - } - } else { - List unfixedFindingsToAdd = - createUnfixedFindingList(path, detectorRule, getKey, getStartLine, issues); - unfixedFindings.addAll(unfixedFindingsToAdd); - } - } - return CodemodFileScanningResult.from(changes, unfixedFindings); - } - - private static @NotNull List createUnfixedFindingList( - final String path, - final DetectorRule detectorRule, - final Function getKey, - final Function getStartLine, - final List issues) { - return issues.stream() - .map( - issue -> - new UnfixedFinding( - getKey.apply(issue), - detectorRule, - path, - getStartLine.apply(issue), - RemediationMessages.ambiguousCodeShape)) - .toList(); - } -} diff --git a/framework/codemodder-base/src/main/java/io/codemodder/remediation/missingsecureflag/MissingSecureFlagFixStrategy.java b/framework/codemodder-base/src/main/java/io/codemodder/remediation/missingsecureflag/MissingSecureFlagFixStrategy.java new file mode 100644 index 000000000..cd319c152 --- /dev/null +++ b/framework/codemodder-base/src/main/java/io/codemodder/remediation/missingsecureflag/MissingSecureFlagFixStrategy.java @@ -0,0 +1,64 @@ +package io.codemodder.remediation.missingsecureflag; + +import com.github.javaparser.ast.CompilationUnit; +import com.github.javaparser.ast.Node; +import com.github.javaparser.ast.NodeList; +import com.github.javaparser.ast.expr.BooleanLiteralExpr; +import com.github.javaparser.ast.expr.Expression; +import com.github.javaparser.ast.expr.MethodCallExpr; +import com.github.javaparser.ast.stmt.ExpressionStmt; +import com.github.javaparser.ast.stmt.Statement; +import io.codemodder.ast.ASTTransforms; +import io.codemodder.remediation.RemediationStrategy; +import io.codemodder.remediation.SuccessOrReason; +import java.util.Optional; + +/** Default strategy to add missing secure flags in cookies. */ +public final class MissingSecureFlagFixStrategy implements RemediationStrategy { + + @Override + public SuccessOrReason fix(final CompilationUnit cu, final Node node) { + var maybeCall = + Optional.of(node) + .map(n -> n instanceof MethodCallExpr ? (MethodCallExpr) n : null) + .filter(Node::hasScope); + + if (maybeCall.isPresent()) { + var methodCallExpr = maybeCall.get(); + + // This assumption is a bit strong, but covers the most common cases while avoiding weird + // ones + Optional maybeStmt = + methodCallExpr + .getParentNode() + .map(p -> p instanceof Statement ? (Statement) p : null) + .filter(Statement::isExpressionStmt); + if (maybeStmt.isEmpty()) { + return SuccessOrReason.reason("Could not find expression statement containing call"); + } + + // We want to avoid things like: response.addCookie(new Cookie(...)), so we restrict + // ourselves + Optional maybeCookieExpression = + methodCallExpr.getArguments().stream() + .findFirst() + .filter(expr -> expr.isNameExpr() || expr.isFieldAccessExpr()); + + if (maybeCookieExpression.isEmpty()) { + return SuccessOrReason.reason("First argument is not a name or field access expression"); + } + + final var newStatement = + new ExpressionStmt( + new MethodCallExpr( + maybeCookieExpression.get(), + "setSecure", + new NodeList<>(new BooleanLiteralExpr(true)))); + + ASTTransforms.addStatementBeforeStatement(maybeStmt.get(), newStatement); + + return SuccessOrReason.success(); + } + return SuccessOrReason.reason("Not a method call with scope."); + } +} diff --git a/framework/codemodder-base/src/main/java/io/codemodder/remediation/missingsecureflag/MissingSecureFlagRemediator.java b/framework/codemodder-base/src/main/java/io/codemodder/remediation/missingsecureflag/MissingSecureFlagRemediator.java index 3cc14c77b..5ba9998dc 100644 --- a/framework/codemodder-base/src/main/java/io/codemodder/remediation/missingsecureflag/MissingSecureFlagRemediator.java +++ b/framework/codemodder-base/src/main/java/io/codemodder/remediation/missingsecureflag/MissingSecureFlagRemediator.java @@ -1,24 +1,53 @@ package io.codemodder.remediation.missingsecureflag; import com.github.javaparser.ast.CompilationUnit; +import com.github.javaparser.ast.expr.MethodCallExpr; import io.codemodder.CodemodFileScanningResult; import io.codemodder.codetf.DetectorRule; -import java.util.List; +import io.codemodder.remediation.*; +import java.util.Collection; +import java.util.Optional; import java.util.function.Function; -/** Strategy interface for remediating missing secure flag vulnerabilities. */ -public interface MissingSecureFlagRemediator { +public final class MissingSecureFlagRemediator implements Remediator { - /** Remediate all missing secure flag vulnerabilities in the given compilation unit. */ - CodemodFileScanningResult remediateAll( + private final SearcherStrategyRemediator searchStrategyRemediator; + + public MissingSecureFlagRemediator() { + this.searchStrategyRemediator = + new SearcherStrategyRemediator.Builder() + .withSearcherStrategyPair( + new FixCandidateSearcher.Builder() + .withMatcher( + node -> + Optional.of(node) + .map(n -> n instanceof MethodCallExpr ? (MethodCallExpr) n : null) + .filter(mce -> "addCookie".equals(mce.getNameAsString())) + .filter(mce -> mce.getArguments().size() == 1) + .isPresent()) + .build(), + new MissingSecureFlagFixStrategy()) + .build(); + } + + @Override + public CodemodFileScanningResult remediateAll( CompilationUnit cu, String path, DetectorRule detectorRule, - List issuesForFile, - Function getKey, - Function getStartLine, - Function getEndLine, - Function getStartColumn); - - MissingSecureFlagRemediator DEFAULT = new DefaultMissingSecureFlagRemediator(); + Collection findingsForPath, + Function findingIdExtractor, + Function findingStartLineExtractor, + Function> findingEndLineExtractor, + Function> findingColumnExtractor) { + return searchStrategyRemediator.remediateAll( + cu, + path, + detectorRule, + findingsForPath, + findingIdExtractor, + findingStartLineExtractor, + findingEndLineExtractor, + findingColumnExtractor); + } } diff --git a/framework/codemodder-base/src/main/java/io/codemodder/remediation/reflectioninjection/DefaultReflectionInjectionRemediator.java b/framework/codemodder-base/src/main/java/io/codemodder/remediation/reflectioninjection/DefaultReflectionInjectionRemediator.java deleted file mode 100644 index 318cc2d37..000000000 --- a/framework/codemodder-base/src/main/java/io/codemodder/remediation/reflectioninjection/DefaultReflectionInjectionRemediator.java +++ /dev/null @@ -1,130 +0,0 @@ -package io.codemodder.remediation.reflectioninjection; - -import static io.codemodder.ast.ASTTransforms.addImportIfMissing; - -import com.github.javaparser.ast.CompilationUnit; -import com.github.javaparser.ast.expr.MethodCallExpr; -import com.github.javaparser.ast.expr.NameExpr; -import io.codemodder.CodemodChange; -import io.codemodder.CodemodFileScanningResult; -import io.codemodder.DependencyGAV; -import io.codemodder.codetf.DetectorRule; -import io.codemodder.codetf.FixedFinding; -import io.codemodder.remediation.LegacyFixCandidate; -import io.codemodder.remediation.LegacyFixCandidateSearchResults; -import io.codemodder.remediation.LegacyFixCandidateSearcher; -import io.codemodder.remediation.MethodOrConstructor; -import io.github.pixee.security.Reflection; -import java.util.ArrayList; -import java.util.List; -import java.util.function.Function; - -final class DefaultReflectionInjectionRemediator implements ReflectionInjectionRemediator { - - @Override - public CodemodFileScanningResult remediateAll( - final CompilationUnit cu, - final String path, - final DetectorRule detectorRule, - final List issuesForFile, - final Function getKey, - final Function getStartLine, - final Function getEndLine, - final Function getStartColumn) { - - LegacyFixCandidateSearcher searcher = - new LegacyFixCandidateSearcher.Builder() - .withMatcher(mce -> isClassForNameCall(cu, mce)) - .build(); - - LegacyFixCandidateSearchResults results = - searcher.search( - cu, - path, - detectorRule, - issuesForFile, - getKey, - getStartLine, - getEndLine, - getStartColumn); - - List changes = new ArrayList<>(); - - for (LegacyFixCandidate legacyFixCandidate : results.fixCandidates()) { - List issues = legacyFixCandidate.issues(); - int line = getStartLine.apply(issues.get(0)); - - MethodCallExpr methodCallExpr = legacyFixCandidate.call().asMethodCall(); - replaceMethodCallExpression(cu, methodCallExpr); - - issues.stream() - .map(getKey) - .forEach( - id -> { - CodemodChange change = - CodemodChange.from( - line, - List.of(DependencyGAV.JAVA_SECURITY_TOOLKIT), - new FixedFinding(id, detectorRule)); - changes.add(change); - }); - } - - return CodemodFileScanningResult.from(changes, results.unfixableFindings()); - } - - /** - * Updates the scope and name of the method call expression to {@code Reflection.loadAndVerify}, - * and adds the import if missing. - * - * @param cu CompilationUnit to update with imports - * @param methodCallExpr the method call expression to update - */ - private static void replaceMethodCallExpression( - final CompilationUnit cu, final MethodCallExpr methodCallExpr) { - final var name = new NameExpr(Reflection.class.getSimpleName()); - methodCallExpr.setScope(name); - methodCallExpr.setName("loadAndVerify"); - addImportIfMissing(cu, Reflection.class); - } - - /** - * Check if the method call expression is a call to {@code Class.forName(String)}. - * - *

This is important, because this codemod uses fuzzy region matching, to account for how Sonar - * reports the region for this finding. Sonar reports the region to be only the "forName" part of - * the method expression. - * - * @param cu CompilationUnit for checking static imports - * @param methodOrConstructor the node to check - * @return true if the method call expression is a call to {@code Class.forName(String)} - */ - private static boolean isClassForNameCall( - final CompilationUnit cu, final MethodOrConstructor methodOrConstructor) { - if (!methodOrConstructor.isMethodCall()) { - return false; - } - MethodCallExpr methodCallExpr = methodOrConstructor.asMethodCall(); - final boolean scopeMatches = - methodCallExpr - .getScope() - .map( - expression -> { - if (expression.isNameExpr()) { - final var nameExpr = expression.asNameExpr(); - return nameExpr.getNameAsString().equals("Class"); - } - return false; - }) - .orElse( - cu.getImports().stream() - .anyMatch( - importDeclaration -> - importDeclaration.isStatic() - && importDeclaration - .getNameAsString() - .equals("java.lang.Class.forName"))); - final var methodNameMatches = methodCallExpr.getNameAsString().equals("forName"); - return scopeMatches && methodNameMatches; - } -} diff --git a/framework/codemodder-base/src/main/java/io/codemodder/remediation/reflectioninjection/ReflectionInjectionFixStrategy.java b/framework/codemodder-base/src/main/java/io/codemodder/remediation/reflectioninjection/ReflectionInjectionFixStrategy.java new file mode 100644 index 000000000..33e081f8a --- /dev/null +++ b/framework/codemodder-base/src/main/java/io/codemodder/remediation/reflectioninjection/ReflectionInjectionFixStrategy.java @@ -0,0 +1,46 @@ +package io.codemodder.remediation.reflectioninjection; + +import static io.codemodder.ast.ASTTransforms.addImportIfMissing; + +import com.github.javaparser.ast.CompilationUnit; +import com.github.javaparser.ast.Node; +import com.github.javaparser.ast.expr.MethodCallExpr; +import com.github.javaparser.ast.expr.NameExpr; +import io.codemodder.DependencyGAV; +import io.codemodder.remediation.RemediationStrategy; +import io.codemodder.remediation.SuccessOrReason; +import io.github.pixee.security.Reflection; +import java.util.List; +import java.util.Optional; + +/** Default strategy to fix reflection injection vulnerabilities. */ +public final class ReflectionInjectionFixStrategy implements RemediationStrategy { + + @Override + public SuccessOrReason fix(final CompilationUnit cu, final Node node) { + var maybeCall = + Optional.of(node).map(n -> n instanceof MethodCallExpr ? (MethodCallExpr) n : null); + if (maybeCall.isEmpty()) { + return SuccessOrReason.reason("Not a method call"); + } + + MethodCallExpr methodCallExpr = maybeCall.get(); + replaceMethodCallExpression(cu, methodCallExpr); + return SuccessOrReason.success(List.of(DependencyGAV.JAVA_SECURITY_TOOLKIT)); + } + + /** + * Updates the scope and name of the method call expression to {@code Reflection.loadAndVerify}, + * and adds the import if missing. + * + * @param cu CompilationUnit to update with imports + * @param methodCallExpr the method call expression to update + */ + private static void replaceMethodCallExpression( + final CompilationUnit cu, final MethodCallExpr methodCallExpr) { + final var name = new NameExpr(Reflection.class.getSimpleName()); + methodCallExpr.setScope(name); + methodCallExpr.setName("loadAndVerify"); + addImportIfMissing(cu, Reflection.class); + } +} diff --git a/framework/codemodder-base/src/main/java/io/codemodder/remediation/reflectioninjection/ReflectionInjectionRemediator.java b/framework/codemodder-base/src/main/java/io/codemodder/remediation/reflectioninjection/ReflectionInjectionRemediator.java index b9012611f..e737c3c93 100644 --- a/framework/codemodder-base/src/main/java/io/codemodder/remediation/reflectioninjection/ReflectionInjectionRemediator.java +++ b/framework/codemodder-base/src/main/java/io/codemodder/remediation/reflectioninjection/ReflectionInjectionRemediator.java @@ -1,24 +1,91 @@ package io.codemodder.remediation.reflectioninjection; import com.github.javaparser.ast.CompilationUnit; +import com.github.javaparser.ast.expr.MethodCallExpr; import io.codemodder.CodemodFileScanningResult; import io.codemodder.codetf.DetectorRule; -import java.util.List; +import io.codemodder.remediation.*; +import java.util.Collection; +import java.util.Optional; import java.util.function.Function; -/** Remediates reflection injection vulnerabilities. */ -public interface ReflectionInjectionRemediator { +public final class ReflectionInjectionRemediator implements Remediator { - /** Remediate all reflection injection vulnerabilities in the given compilation unit. */ - CodemodFileScanningResult remediateAll( + private final SearcherStrategyRemediator searchStrategyRemediator; + + public ReflectionInjectionRemediator() { + this.searchStrategyRemediator = + new SearcherStrategyRemediator.Builder() + .withSearcherStrategyPair( + new FixCandidateSearcher.Builder() + .withMatcher( + node -> + Optional.of(node) + .map(n -> n instanceof MethodCallExpr ? (MethodCallExpr) n : null) + .filter(ReflectionInjectionRemediator::isClassForNameCall) + .isPresent()) + .build(), + new ReflectionInjectionFixStrategy()) + .build(); + } + + @Override + public CodemodFileScanningResult remediateAll( CompilationUnit cu, String path, DetectorRule detectorRule, - List issuesForFile, - Function getKey, - Function getStartLine, - Function getEndLine, - Function getColumn); + Collection findingsForPath, + Function findingIdExtractor, + Function findingStartLineExtractor, + Function> findingEndLineExtractor, + Function> findingColumnExtractor) { + return searchStrategyRemediator.remediateAll( + cu, + path, + detectorRule, + findingsForPath, + findingIdExtractor, + findingStartLineExtractor, + findingEndLineExtractor, + findingColumnExtractor); + } - ReflectionInjectionRemediator DEFAULT = new DefaultReflectionInjectionRemediator(); + /** + * Check if the method call expression is a call to {@code Class.forName(String)}. + * + *

This is important, because this codemod uses fuzzy region matching, to account for how Sonar + * reports the region for this finding. Sonar reports the region to be only the "forName" part of + * the method expression. + * + * @param methodCallExpr the node to check + * @return true if the method call expression is a call to {@code Class.forName(String)} + */ + private static boolean isClassForNameCall(final MethodCallExpr methodCallExpr) { + var maybeCU = methodCallExpr.findCompilationUnit(); + if (maybeCU.isEmpty()) { + return false; + } + var cu = maybeCU.get(); + final boolean scopeMatches = + methodCallExpr + .getScope() + .map( + expression -> { + if (expression.isNameExpr()) { + final var nameExpr = expression.asNameExpr(); + return nameExpr.getNameAsString().equals("Class"); + } + return false; + }) + .orElse( + cu.getImports().stream() + .anyMatch( + importDeclaration -> + importDeclaration.isStatic() + && importDeclaration + .getNameAsString() + .equals("java.lang.Class.forName"))); + final var methodNameMatches = methodCallExpr.getNameAsString().equals("forName"); + return scopeMatches && methodNameMatches; + } } diff --git a/framework/codemodder-base/src/main/java/io/codemodder/remediation/sqlinjection/SQLInjectionFixComposer.java b/framework/codemodder-base/src/main/java/io/codemodder/remediation/sqlinjection/SQLInjectionFixComposer.java index 1fb45757b..ef48496c3 100644 --- a/framework/codemodder-base/src/main/java/io/codemodder/remediation/sqlinjection/SQLInjectionFixComposer.java +++ b/framework/codemodder-base/src/main/java/io/codemodder/remediation/sqlinjection/SQLInjectionFixComposer.java @@ -24,7 +24,7 @@ public SQLInjectionFixComposer() {} * @param binaryExpr * @return An Optional containing the execute call if successful */ - private Optional flowsIntoExecuteCall(final BinaryExpr binaryExpr) { + private Optional flowsIntoExecuteCall(final Expression binaryExpr) { // Is argument of a method call var maybeDirectArgumentOfCall = ASTs.isArgumentOfMethodCall(binaryExpr).filter(SQLInjectionFixComposer::match); @@ -58,10 +58,10 @@ private Optional isIndirectArgumentOfCall(final Expression expr) public SuccessOrReason fix(final CompilationUnit cu, final Node node) { // Is a binary expr or method call expr? - Optional> morb; + Optional> morb; if (node instanceof MethodCallExpr m) { morb = Optional.of(Either.left(m)); - } else if (node instanceof BinaryExpr b) { + } else if (node instanceof Expression b) { morb = Optional.of(Either.right(b)); } else { morb = Optional.empty(); diff --git a/framework/codemodder-base/src/main/java/io/codemodder/remediation/sqlinjection/SQLInjectionRemediator.java b/framework/codemodder-base/src/main/java/io/codemodder/remediation/sqlinjection/SQLInjectionRemediator.java index f043745a6..efab4325d 100644 --- a/framework/codemodder-base/src/main/java/io/codemodder/remediation/sqlinjection/SQLInjectionRemediator.java +++ b/framework/codemodder-base/src/main/java/io/codemodder/remediation/sqlinjection/SQLInjectionRemediator.java @@ -21,7 +21,11 @@ public SQLInjectionRemediator() { .withSearcherStrategyPair( new FixCandidateSearcher.Builder() .withMatcher( - n -> Optional.of(n).filter(SQLInjectionFixComposer::match).isPresent()) + n -> + Optional.empty() + // is the call itself + .or(() -> Optional.of(n).filter(SQLInjectionFixComposer::match)) + .isPresent()) .build(), new SQLInjectionFixComposer()) .build(); diff --git a/framework/codemodder-base/src/main/java/io/codemodder/remediation/ssrf/DefaultSSRFRemediator.java b/framework/codemodder-base/src/main/java/io/codemodder/remediation/ssrf/DefaultSSRFRemediator.java deleted file mode 100644 index 52d9f5940..000000000 --- a/framework/codemodder-base/src/main/java/io/codemodder/remediation/ssrf/DefaultSSRFRemediator.java +++ /dev/null @@ -1,211 +0,0 @@ -package io.codemodder.remediation.ssrf; - -import static io.codemodder.ast.ASTTransforms.addImportIfMissing; - -import com.github.javaparser.ast.CompilationUnit; -import com.github.javaparser.ast.NodeList; -import com.github.javaparser.ast.expr.Expression; -import com.github.javaparser.ast.expr.FieldAccessExpr; -import com.github.javaparser.ast.expr.MethodCallExpr; -import com.github.javaparser.ast.expr.NameExpr; -import com.github.javaparser.ast.expr.ObjectCreationExpr; -import io.codemodder.CodemodChange; -import io.codemodder.CodemodFileScanningResult; -import io.codemodder.DependencyGAV; -import io.codemodder.codetf.DetectorRule; -import io.codemodder.codetf.FixedFinding; -import io.codemodder.codetf.UnfixedFinding; -import io.codemodder.remediation.LegacyFixCandidate; -import io.codemodder.remediation.LegacyFixCandidateSearchResults; -import io.codemodder.remediation.LegacyFixCandidateSearcher; -import io.codemodder.remediation.MethodOrConstructor; -import io.github.pixee.security.HostValidator; -import io.github.pixee.security.Urls; -import java.util.ArrayList; -import java.util.List; -import java.util.function.BiPredicate; -import java.util.function.Function; -import org.javatuples.Pair; - -final class DefaultSSRFRemediator implements SSRFRemediator { - - @Override - public CodemodFileScanningResult remediateAll( - final CompilationUnit cu, - final String path, - final DetectorRule detectorRule, - final List issuesForFile, - final Function getKey, - final Function getStartLine, - final Function getEndLine, - final Function getStartColumn) { - - // search for new URL() calls on those lines, assuming the tool points there -- there are plenty - // of more signatures to chase down - LegacyFixCandidateSearcher searcher = - new LegacyFixCandidateSearcher.Builder() - .withMatcher(mce -> mce.isConstructorForType("URL")) - .withMatcher(mce -> !mce.getArguments().isEmpty()) - .build(); - - // Matches calls like RestTemplate.exchange(...) - // Doesn't actually check that the `exchange` call scope is actually of type RestTemplate - // This is left for the detectors, for now - LegacyFixCandidateSearcher rtSearcher = - new LegacyFixCandidateSearcher.Builder() - // is method with name - .withMatcher(mce -> mce.isMethodCallWithName("exchange")) - // has a scope - .withMatcher(MethodOrConstructor::isMethodCallWithScope) - // Could be improved further by adding a RestTemplate type check to the scope - .build(); - - List changes = new ArrayList<>(); - List unfixedFindings = new ArrayList<>(); - - var pairResult = - searchAndFix( - searcher, - (cunit, moc) -> harden(cunit, moc.asObjectCreationExpr()), - cu, - path, - detectorRule, - issuesForFile, - getKey, - getStartLine, - getEndLine, - getStartColumn); - changes.addAll(pairResult.getValue0()); - unfixedFindings.addAll(pairResult.getValue1()); - - var pairResultRT = - searchAndFix( - rtSearcher, - (cunit, moc) -> hardenRT(cunit, moc.asMethodCall()), - cu, - path, - detectorRule, - issuesForFile, - getKey, - getStartLine, - getEndLine, - getStartColumn); - changes.addAll(pairResultRT.getValue0()); - unfixedFindings.addAll(pairResultRT.getValue1()); - - return CodemodFileScanningResult.from(changes, unfixedFindings); - } - - private boolean harden(final CompilationUnit cu, final ObjectCreationExpr newUrlCall) { - NodeList arguments = newUrlCall.getArguments(); - - /* - * We need to replace: - * - * URL u = new URL(foo) - * - * With: - * - * import io.github.pixee.security.Urls; - * ... - * URL u = Urls.create(foo, io.github.pixee.security.Urls.HTTP_PROTOCOLS, io.github.pixee.security.HostValidator.ALLOW_ALL) - */ - MethodCallExpr safeCall = wrapInUrlsCreate(cu, arguments); - newUrlCall.replace(safeCall); - return true; - } - - private MethodCallExpr wrapInUrlsCreate( - final CompilationUnit cu, final NodeList arguments) { - addImportIfMissing(cu, Urls.class.getName()); - addImportIfMissing(cu, HostValidator.class.getName()); - - FieldAccessExpr httpProtocolsExpr = new FieldAccessExpr(); - httpProtocolsExpr.setScope(new NameExpr(Urls.class.getSimpleName())); - httpProtocolsExpr.setName("HTTP_PROTOCOLS"); - - FieldAccessExpr denyCommonTargetsExpr = new FieldAccessExpr(); - denyCommonTargetsExpr.setScope(new NameExpr(HostValidator.class.getSimpleName())); - denyCommonTargetsExpr.setName("DENY_COMMON_INFRASTRUCTURE_TARGETS"); - - NodeList newArguments = new NodeList<>(); - newArguments.addAll(arguments); // add expression - newArguments.add(httpProtocolsExpr); // load the protocols they're allowed - newArguments.add(denyCommonTargetsExpr); // load the host validator - - return new MethodCallExpr(new NameExpr(Urls.class.getSimpleName()), "create", newArguments); - } - - private boolean hardenRT(final CompilationUnit cu, final MethodCallExpr call) { - var maybeFirstArg = call.getArguments().stream().findFirst(); - if (maybeFirstArg.isPresent()) { - var wrappedArg = - new MethodCallExpr( - wrapInUrlsCreate(cu, new NodeList<>(maybeFirstArg.get().clone())), "toString"); - maybeFirstArg.get().replace(wrappedArg); - return true; - } - return false; - } - - /** - * Returns a list of changes and unfixed findings for a pair of searcher, that gather relevant - * issues, and a fixer predicate, that returns true if the change is successful. - */ - private Pair, List> searchAndFix( - final LegacyFixCandidateSearcher searcher, - final BiPredicate fixer, - final CompilationUnit cu, - final String path, - final DetectorRule detectorRule, - final List issuesForFile, - final Function getKey, - final Function getStartLine, - final Function getEndLine, - final Function getStartColumn) { - List changes = new ArrayList<>(); - List unfixedFindings = new ArrayList<>(); - - LegacyFixCandidateSearchResults results = - searcher.search( - cu, - path, - detectorRule, - issuesForFile, - getKey, - getStartLine, - getEndLine, - getStartColumn); - - for (LegacyFixCandidate candidate : results.fixCandidates()) { - MethodOrConstructor call = candidate.call(); - List issues = candidate.issues(); - if (fixer.test(cu, call)) { - List fixedFindings = - issues.stream() - .map(issue -> new FixedFinding(getKey.apply(issue), detectorRule)) - .toList(); - CodemodChange change = - CodemodChange.from( - getStartLine.apply(issues.get(0)), - List.of(DependencyGAV.JAVA_SECURITY_TOOLKIT), - 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); - } -} diff --git a/framework/codemodder-base/src/main/java/io/codemodder/remediation/ssrf/SSRFFixStrategy.java b/framework/codemodder-base/src/main/java/io/codemodder/remediation/ssrf/SSRFFixStrategy.java new file mode 100644 index 000000000..721ae1534 --- /dev/null +++ b/framework/codemodder-base/src/main/java/io/codemodder/remediation/ssrf/SSRFFixStrategy.java @@ -0,0 +1,95 @@ +package io.codemodder.remediation.ssrf; + +import static io.codemodder.ast.ASTTransforms.addImportIfMissing; + +import com.github.javaparser.ast.CompilationUnit; +import com.github.javaparser.ast.Node; +import com.github.javaparser.ast.NodeList; +import com.github.javaparser.ast.expr.*; +import io.codemodder.DependencyGAV; +import io.codemodder.remediation.RemediationStrategy; +import io.codemodder.remediation.SuccessOrReason; +import io.github.pixee.security.HostValidator; +import io.github.pixee.security.Urls; +import java.util.List; + +/** Default strategy to fix SSRF issues. */ +public final class SSRFFixStrategy implements RemediationStrategy { + + @Override + public SuccessOrReason fix(final CompilationUnit cu, final Node node) { + if (node instanceof MethodCallExpr mce) { + return hardenRT(cu, mce); + + } else if (node instanceof ObjectCreationExpr oce) { + return harden(cu, oce); + } + return SuccessOrReason.reason("Not a method call or constructor"); + } + + /** + * Fixes SSRF issues originating from URL(...) constructors + * + * @param cu + * @param newUrlCall + * @return + */ + private SuccessOrReason harden(final CompilationUnit cu, final ObjectCreationExpr newUrlCall) { + NodeList arguments = newUrlCall.getArguments(); + + /* + * We need to replace: + * + * URL u = new URL(foo) + * + * With: + * + * import io.github.pixee.security.Urls; + * ... + * URL u = Urls.create(foo, io.github.pixee.security.Urls.HTTP_PROTOCOLS, io.github.pixee.security.HostValidator.ALLOW_ALL) + */ + MethodCallExpr safeCall = wrapInUrlsCreate(cu, arguments); + newUrlCall.replace(safeCall); + return SuccessOrReason.success(List.of(DependencyGAV.JAVA_SECURITY_TOOLKIT)); + } + + private MethodCallExpr wrapInUrlsCreate( + final CompilationUnit cu, final NodeList arguments) { + addImportIfMissing(cu, Urls.class.getName()); + addImportIfMissing(cu, HostValidator.class.getName()); + + FieldAccessExpr httpProtocolsExpr = new FieldAccessExpr(); + httpProtocolsExpr.setScope(new NameExpr(Urls.class.getSimpleName())); + httpProtocolsExpr.setName("HTTP_PROTOCOLS"); + + FieldAccessExpr denyCommonTargetsExpr = new FieldAccessExpr(); + denyCommonTargetsExpr.setScope(new NameExpr(HostValidator.class.getSimpleName())); + denyCommonTargetsExpr.setName("DENY_COMMON_INFRASTRUCTURE_TARGETS"); + + NodeList newArguments = new NodeList<>(); + newArguments.addAll(arguments); // add expression + newArguments.add(httpProtocolsExpr); // load the protocols they're allowed + newArguments.add(denyCommonTargetsExpr); // load the host validator + + return new MethodCallExpr(new NameExpr(Urls.class.getSimpleName()), "create", newArguments); + } + + /** + * Fixes SRRF issues originating from RestTemplate.exchange() calls. + * + * @param cu + * @param call + * @return + */ + private SuccessOrReason hardenRT(final CompilationUnit cu, final MethodCallExpr call) { + var maybeFirstArg = call.getArguments().stream().findFirst(); + if (maybeFirstArg.isPresent()) { + var wrappedArg = + new MethodCallExpr( + wrapInUrlsCreate(cu, new NodeList<>(maybeFirstArg.get().clone())), "toString"); + maybeFirstArg.get().replace(wrappedArg); + return SuccessOrReason.success(List.of(DependencyGAV.JAVA_SECURITY_TOOLKIT)); + } + return SuccessOrReason.reason("Could not find first argument"); + } +} diff --git a/framework/codemodder-base/src/main/java/io/codemodder/remediation/ssrf/SSRFRemediator.java b/framework/codemodder-base/src/main/java/io/codemodder/remediation/ssrf/SSRFRemediator.java index 8469f611c..c33b45d06 100644 --- a/framework/codemodder-base/src/main/java/io/codemodder/remediation/ssrf/SSRFRemediator.java +++ b/framework/codemodder-base/src/main/java/io/codemodder/remediation/ssrf/SSRFRemediator.java @@ -1,25 +1,75 @@ package io.codemodder.remediation.ssrf; import com.github.javaparser.ast.CompilationUnit; +import com.github.javaparser.ast.Node; +import com.github.javaparser.ast.expr.MethodCallExpr; +import com.github.javaparser.ast.expr.ObjectCreationExpr; import io.codemodder.CodemodFileScanningResult; +import io.codemodder.Either; import io.codemodder.codetf.DetectorRule; -import java.util.List; +import io.codemodder.remediation.*; +import java.util.Collection; +import java.util.Optional; import java.util.function.Function; -/** Fixes SSRF vulnerabilities. */ -public interface SSRFRemediator { +public final class SSRFRemediator implements Remediator { - /** A default implementation for callers. */ - SSRFRemediator DEFAULT = new DefaultSSRFRemediator(); + private final SearcherStrategyRemediator searchStrategyRemediator; - /** Remediate all SSRF vulnerabilities in the given compilation unit. */ - CodemodFileScanningResult remediateAll( + public SSRFRemediator() { + this.searchStrategyRemediator = + new SearcherStrategyRemediator.Builder() + .withSearcherStrategyPair( + new FixCandidateSearcher.Builder() + .withMatcher( + n -> + Optional.>empty() + .or( + () -> + Optional.of(n) + .map( + m -> + m instanceof ObjectCreationExpr + ? (ObjectCreationExpr) m + : null) + .filter(m -> "URL".equals(m.getTypeAsString())) + .filter(m -> m.getArguments().isNonEmpty()) + .map(Either::right)) + .or( + () -> + Optional.of(n) + .map( + m -> + m instanceof MethodCallExpr + ? (MethodCallExpr) m + : null) + .filter(Node::hasScope) + .filter(m -> "exchange".equals(m.getNameAsString())) + .map(Either::left)) + .isPresent()) + .build(), + new SSRFFixStrategy()) + .build(); + } + + @Override + public CodemodFileScanningResult remediateAll( CompilationUnit cu, String path, DetectorRule detectorRule, - List issuesForFile, - Function getKey, - Function getStartLine, - Function getEndLine, - Function getColumn); + Collection findingsForPath, + Function findingIdExtractor, + Function findingStartLineExtractor, + Function> findingEndLineExtractor, + Function> findingColumnExtractor) { + return searchStrategyRemediator.remediateAll( + cu, + path, + detectorRule, + findingsForPath, + findingIdExtractor, + findingStartLineExtractor, + findingEndLineExtractor, + findingColumnExtractor); + } } diff --git a/framework/codemodder-base/src/test/java/io/codemodder/remediation/headerinjection/DefaultHeaderInjectionRemediatorTest.java b/framework/codemodder-base/src/test/java/io/codemodder/remediation/headerinjection/DefaultHeaderInjectionRemediatorTest.java index 2bbf3e621..7d0a0381f 100644 --- a/framework/codemodder-base/src/test/java/io/codemodder/remediation/headerinjection/DefaultHeaderInjectionRemediatorTest.java +++ b/framework/codemodder-base/src/test/java/io/codemodder/remediation/headerinjection/DefaultHeaderInjectionRemediatorTest.java @@ -12,6 +12,7 @@ import io.codemodder.codetf.UnfixedFinding; import io.codemodder.remediation.RemediationMessages; import java.util.List; +import java.util.Optional; import java.util.stream.Stream; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -21,12 +22,12 @@ final class DefaultHeaderInjectionRemediatorTest { - private DefaultHeaderInjectionRemediator remediator; + private HeaderInjectionRemediator remediator; private DetectorRule rule; @BeforeEach void setup() { - this.remediator = new DefaultHeaderInjectionRemediator(); + this.remediator = new HeaderInjectionRemediator<>(); this.rule = new DetectorRule("header-injection", "Header Injection", null); } @@ -52,8 +53,8 @@ void it_doesnt_fix_unfixable(final String unfixableCode, final int line, final S List.of(finding), f -> f.id, f -> line, - f -> null, - f -> null); + f -> Optional.empty(), + f -> Optional.empty()); assertThat(result.changes()).isEmpty(); assertThat(result.unfixedFindings()).hasSize(1); UnfixedFinding unfixedFinding = result.unfixedFindings().get(0); @@ -186,8 +187,8 @@ void it_fixes_header_injection( List.of(finding), f -> f.id, f -> f.line, - f -> null, - f -> null); + f -> Optional.empty(), + f -> Optional.empty()); assertThat(result.changes()).hasSize(1); CodemodChange change = result.changes().get(0); assertThat(change.lineNumber()).isEqualTo(line); diff --git a/framework/codemodder-base/src/test/java/io/codemodder/remediation/javadeserialization/DefaultJavaDeserializationRemediatorTest.java b/framework/codemodder-base/src/test/java/io/codemodder/remediation/javadeserialization/DefaultJavaDeserializationRemediatorTest.java index 2d9a6e1c9..666142323 100644 --- a/framework/codemodder-base/src/test/java/io/codemodder/remediation/javadeserialization/DefaultJavaDeserializationRemediatorTest.java +++ b/framework/codemodder-base/src/test/java/io/codemodder/remediation/javadeserialization/DefaultJavaDeserializationRemediatorTest.java @@ -12,6 +12,7 @@ import io.codemodder.codetf.FixedFinding; import io.codemodder.codetf.UnfixedFinding; import java.util.List; +import java.util.Optional; import java.util.stream.Stream; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.params.ParameterizedTest; @@ -21,12 +22,12 @@ final class DefaultJavaDeserializationRemediatorTest { - private DefaultJavaDeserializationRemediator remediator; + private JavaDeserializationRemediator remediator; private DetectorRule rule; @BeforeEach void setup() { - remediator = new DefaultJavaDeserializationRemediator(); + remediator = new JavaDeserializationRemediator<>(); rule = new DetectorRule("untrusted-deserialization", "Untrusted Deserialization", null); } @@ -64,7 +65,14 @@ void it_doesnt_handle_unfixables(final String badCode, final int line, final Str CodemodFileScanningResult result = remediator.remediateAll( - cu, "path", rule, List.of(new Object()), o -> "id", o -> line, o -> null, o -> null); + cu, + "path", + rule, + List.of(new Object()), + o -> "id", + o -> line, + o -> Optional.empty(), + o -> Optional.empty()); assertThat(result.unfixedFindings()).hasSize(1); assertThat(result.changes()).isEmpty(); UnfixedFinding unfixedFinding = result.unfixedFindings().get(0); @@ -103,7 +111,14 @@ void it_fixes_java_deserialization(final int line) { CodemodFileScanningResult result = remediator.remediateAll( - cu, "path", rule, List.of(new Object()), o -> "id", o -> line, o -> null, o -> null); + cu, + "path", + rule, + List.of(new Object()), + o -> "id", + o -> line, + o -> Optional.empty(), + o -> Optional.empty()); assertThat(result.unfixedFindings()).isEmpty(); assertThat(result.changes()).hasSize(1); CodemodChange change = result.changes().get(0); diff --git a/framework/codemodder-base/src/test/java/io/codemodder/remediation/missingsecureflag/DefaultMissingSecureFlagRemediatorTest.java b/framework/codemodder-base/src/test/java/io/codemodder/remediation/missingsecureflag/DefaultMissingSecureFlagRemediatorTest.java index c22613249..1be9ac8d5 100644 --- a/framework/codemodder-base/src/test/java/io/codemodder/remediation/missingsecureflag/DefaultMissingSecureFlagRemediatorTest.java +++ b/framework/codemodder-base/src/test/java/io/codemodder/remediation/missingsecureflag/DefaultMissingSecureFlagRemediatorTest.java @@ -10,17 +10,18 @@ import io.codemodder.codetf.DetectorRule; import io.codemodder.codetf.FixedFinding; import java.util.List; +import java.util.Optional; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ValueSource; final class DefaultMissingSecureFlagRemediatorTest { - private DefaultMissingSecureFlagRemediator remediator; + private MissingSecureFlagRemediator remediator; @BeforeEach void setup() { - remediator = new DefaultMissingSecureFlagRemediator(); + remediator = new MissingSecureFlagRemediator<>(); } @ParameterizedTest @@ -68,8 +69,8 @@ void it_does_not_add_secure_flag(final String javaCode) { List.of(new Object()), r -> "id-1", r -> 5, - r -> null, - r -> null); + r -> Optional.empty(), + r -> Optional.empty()); assertThat(result.changes()).isEmpty(); result @@ -108,8 +109,8 @@ void doGet(HttpServletRequest request, HttpServletResponse response) { List.of(new Object()), r -> "id-1", r -> line, - r -> null, - r -> null); + r -> Optional.empty(), + r -> Optional.empty()); assertThat(result.unfixedFindings()).isEmpty(); List changes = result.changes(); diff --git a/framework/codemodder-base/src/test/java/io/codemodder/remediation/reflectioninjection/DefaultReflectionInjectionRemediatorTest.java b/framework/codemodder-base/src/test/java/io/codemodder/remediation/reflectioninjection/DefaultReflectionInjectionRemediatorTest.java index 88cc0e1fc..4ab16fdfe 100644 --- a/framework/codemodder-base/src/test/java/io/codemodder/remediation/reflectioninjection/DefaultReflectionInjectionRemediatorTest.java +++ b/framework/codemodder-base/src/test/java/io/codemodder/remediation/reflectioninjection/DefaultReflectionInjectionRemediatorTest.java @@ -11,22 +11,21 @@ import io.codemodder.codetf.DetectorRule; import io.codemodder.remediation.RemediationMessages; import java.util.List; +import java.util.Optional; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; -/** - * Unit tests for {@link DefaultReflectionInjectionRemediator} that use a fake detection input type. - */ +/** Unit tests for {@link ReflectionInjectionRemediator} that use a fake detection input type. */ final class DefaultReflectionInjectionRemediatorTest { - private DefaultReflectionInjectionRemediator remediator; + private ReflectionInjectionRemediator remediator; private DetectorRule rule; @BeforeEach void setup() { - this.remediator = new DefaultReflectionInjectionRemediator(); + this.remediator = new ReflectionInjectionRemediator<>(); this.rule = new DetectorRule("reflection-injection", "Reflection Injection", null); } @@ -201,8 +200,8 @@ private void verifyFixable( List.of(finding), ReflectionInjectionFinding::key, ReflectionInjectionFinding::line, - f -> null, - ReflectionInjectionFinding::column); + f -> Optional.empty(), + f -> Optional.ofNullable(f.column)); assertThat(result.unfixedFindings()).isEmpty(); assertThat(result.changes()).hasSize(1); @@ -270,8 +269,8 @@ private void verifyUnfixable( List.of(finding), ReflectionInjectionFinding::key, ReflectionInjectionFinding::line, - f -> null, - ReflectionInjectionFinding::column); + f -> Optional.empty(), + f -> Optional.ofNullable(f.column)); assertThat(result.changes()).isEmpty(); assertThat(result.unfixedFindings()).hasSize(1); assertThat(result.unfixedFindings().get(0).getLine()).isEqualTo(line); diff --git a/plugins/codemodder-plugin-appscan/src/main/java/io/codemodder/providers/sarif/appscan/AppScanRuleSarif.java b/plugins/codemodder-plugin-appscan/src/main/java/io/codemodder/providers/sarif/appscan/AppScanRuleSarif.java index a46cb928d..93fd6ebab 100644 --- a/plugins/codemodder-plugin-appscan/src/main/java/io/codemodder/providers/sarif/appscan/AppScanRuleSarif.java +++ b/plugins/codemodder-plugin-appscan/src/main/java/io/codemodder/providers/sarif/appscan/AppScanRuleSarif.java @@ -81,7 +81,7 @@ public List getResultsByLocationPath(final Path path) { result -> artifactLocationIndices.get(path) != null && artifactLocationIndices - .get(path) + .getOrDefault(path, Set.of()) .contains( result .getLocations()