-
Notifications
You must be signed in to change notification settings - Fork 7
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Add ability to remediate other XSS code shapes (#481)
Took logic specific to Semgrep and generalized.
- Loading branch information
Showing
7 changed files
with
212 additions
and
40 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
48 changes: 48 additions & 0 deletions
48
...dder-base/src/main/java/io/codemodder/remediation/xss/ResponseEntityWriteFixStrategy.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,48 @@ | ||
| package io.codemodder.remediation.xss; | ||
|
|
||
| import com.github.javaparser.ast.CompilationUnit; | ||
| import com.github.javaparser.ast.Node; | ||
| import com.github.javaparser.ast.expr.Expression; | ||
| import com.github.javaparser.ast.expr.MethodCallExpr; | ||
| import com.github.javaparser.resolution.types.ResolvedType; | ||
| import io.codemodder.remediation.RemediationStrategy; | ||
| import io.codemodder.remediation.SuccessOrReason; | ||
| import java.util.Optional; | ||
|
|
||
| /** | ||
| * Fix strategy for XSS vulnerabilities where a variable/expr is sent to a Spring ResponseEntity | ||
| * write method like ok(). | ||
| */ | ||
| final class ResponseEntityWriteFixStrategy 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 call = maybeCall.get(); | ||
| return EncoderWrapping.fix(call, 0); | ||
| } | ||
|
|
||
| static boolean match(final Node node) { | ||
| return Optional.of(node) | ||
| .map(n -> n instanceof MethodCallExpr ? (MethodCallExpr) n : null) | ||
| .filter(m -> "ok".equals(m.getNameAsString())) | ||
| .filter(m -> !m.getArguments().isEmpty()) | ||
| .filter( | ||
| c -> { | ||
| Expression firstArg = c.getArguments().getFirst().get(); | ||
| try { | ||
| ResolvedType resolvedType = firstArg.calculateResolvedType(); | ||
| return "java.lang.String".equals(resolvedType.describe()); | ||
| } catch (Exception e) { | ||
| // this is expected often, and indicates its a non-String type anyway | ||
| return false; | ||
| } | ||
| }) | ||
| .isPresent(); | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
127 changes: 127 additions & 0 deletions
127
...-base/src/test/java/io/codemodder/remediation/xss/ResponseEntityWriteFixStrategyTest.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,127 @@ | ||
| package io.codemodder.remediation.xss; | ||
|
|
||
| import static org.assertj.core.api.Assertions.assertThat; | ||
|
|
||
| import com.github.javaparser.JavaParser; | ||
| import com.github.javaparser.ast.CompilationUnit; | ||
| import com.github.javaparser.printer.lexicalpreservation.LexicalPreservingPrinter; | ||
| import io.codemodder.CodemodFileScanningResult; | ||
| import io.codemodder.codetf.DetectorRule; | ||
| import io.codemodder.javaparser.JavaParserFactory; | ||
| import io.codemodder.remediation.FixCandidateSearcher; | ||
| import io.codemodder.remediation.SearcherStrategyRemediator; | ||
| import java.io.IOException; | ||
| 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; | ||
| import org.junit.jupiter.params.provider.Arguments; | ||
| import org.junit.jupiter.params.provider.MethodSource; | ||
|
|
||
| final class ResponseEntityWriteFixStrategyTest { | ||
|
|
||
| private ResponseEntityWriteFixStrategy fixer; | ||
| private DetectorRule rule; | ||
| private JavaParser parser; | ||
|
|
||
| @BeforeEach | ||
| void setup() throws IOException { | ||
| this.fixer = new ResponseEntityWriteFixStrategy(); | ||
| this.parser = JavaParserFactory.newFactory().create(List.of()); | ||
| this.rule = new DetectorRule("xss", "XSS", null); | ||
| } | ||
|
|
||
| private static Stream<Arguments> fixableSamples() { | ||
| return Stream.of( | ||
| Arguments.of( | ||
| """ | ||
| class Samples { | ||
| String should_be_fixed(String s) { | ||
| return ResponseEntity.ok("Value: " + s); | ||
| } | ||
| } | ||
| """, | ||
| """ | ||
| import org.owasp.encoder.Encode; | ||
| class Samples { | ||
| String should_be_fixed(String s) { | ||
| return ResponseEntity.ok("Value: " + Encode.forHtml(s)); | ||
| } | ||
| } | ||
| """), | ||
| Arguments.of( | ||
| """ | ||
| class Samples { | ||
| String should_be_fixed(Object s) { | ||
| return ResponseEntity.ok("Value: " + s.toString()); | ||
| } | ||
| } | ||
| """, | ||
| """ | ||
| import org.owasp.encoder.Encode; | ||
| class Samples { | ||
| String should_be_fixed(Object s) { | ||
| return ResponseEntity.ok("Value: " + Encode.forHtml(s.toString())); | ||
| } | ||
| } | ||
| """)); | ||
| } | ||
|
|
||
| @ParameterizedTest | ||
| @MethodSource("fixableSamples") | ||
| void it_fixes_obvious_response_write_methods(final String beforeCode, final String afterCode) { | ||
| CompilationUnit cu = parser.parse(beforeCode).getResult().orElseThrow(); | ||
| LexicalPreservingPrinter.setup(cu); | ||
|
|
||
| var result = scanAndFix(cu, 3); | ||
| assertThat(result.changes()).isNotEmpty(); | ||
| String actualCode = LexicalPreservingPrinter.print(cu); | ||
| assertThat(actualCode).isEqualToIgnoringWhitespace(afterCode); | ||
| } | ||
|
|
||
| private CodemodFileScanningResult scanAndFix(final CompilationUnit cu, final int line) { | ||
| XSSFinding finding = new XSSFinding("should_be_fixed", line, null); | ||
| var remediator = | ||
| new SearcherStrategyRemediator.Builder<XSSFinding>() | ||
| .withSearcherStrategyPair( | ||
| new FixCandidateSearcher.Builder<XSSFinding>() | ||
| .withMatcher(ResponseEntityWriteFixStrategy::match) | ||
| .build(), | ||
| fixer) | ||
| .build(); | ||
| return remediator.remediateAll( | ||
| cu, | ||
| "path", | ||
| rule, | ||
| List.of(finding), | ||
| XSSFinding::key, | ||
| XSSFinding::line, | ||
| x -> Optional.empty(), | ||
| x -> Optional.ofNullable(x.column())); | ||
| } | ||
|
|
||
| @ParameterizedTest | ||
| @MethodSource("unfixableSamples") | ||
| void it_does_not_fix_unfixable_samples(final String beforeCode, final int line) { | ||
| CompilationUnit cu = parser.parse(beforeCode).getResult().orElseThrow(); | ||
| LexicalPreservingPrinter.setup(cu); | ||
| var result = scanAndFix(cu, line); | ||
| assertThat(result.changes()).isEmpty(); | ||
| } | ||
|
|
||
| private static Stream<Arguments> unfixableSamples() { | ||
| return Stream.of( | ||
| // this is not a ResponseEntity, shouldn't touch it | ||
| Arguments.of( | ||
| // this is not a ResponseEntity, shouldn't touch it | ||
| """ | ||
| class Samples { | ||
| String should_be_fixed(String s) { | ||
| return ResponseEntity.something_besides_ok("Value: " + s); | ||
| } | ||
| } | ||
| """, | ||
| 3)); | ||
| } | ||
| } |