Skip to content

Commit 35978f5

Browse files
nahsranahsra
authored
Build generic XSS remediator (#410)
Also included some Semgrep maintenance to make this easier to test in downstream projects.
1 parent 08a0915 commit 35978f5

File tree

18 files changed

+641
-8
lines changed

18 files changed

+641
-8
lines changed

‎core-codemods/src/main/resources/io/codemodder/codemods/JSPScriptletXSSCodemod/description.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ Our changes introduce an HTML-encoding mechanism that look something like this:
1212

1313
```diff
1414
- Welcome to our site <%= request.getParameter("name") %>
15-
+ Welcome to our site <%= io.github.pixee.security.HtmlEncoder.encode(request.getParameter("name")) %>
15+
+ Welcome to our site <%= org.owasp.encoder.Encode.forHtml(request.getParameter("name")) %>
1616
```
1717

1818
This change neutralizes the control characters that attackers would use to execute code. Depending on the context in which the scriptlet is rendered (e.g., inside HTML tags, HTML attributes, in JavaScript, quoted contexts, etc.), you may need to use another encoder. Check out the [OWASP XSS Prevention CheatSheet](https://cheatsheetseries.owasp.org/cheatsheets/Cross_Site_Scripting_Prevention_Cheat_Sheet.html) to learn more about these cases and other controls you may need.

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
/** Reporter strategies that are useful without mentioning specific APIs. */
77
public enum GenericRemediationMetadata {
88
XXE("xxe"),
9+
XSS("xss"),
910
JNDI("jndi-injection"),
1011
HEADER_INJECTION("header-injection"),
1112
REFLECTION_INJECTION("reflection-injection"),

‎framework/codemodder-base/src/main/java/io/codemodder/remediation/RemediationMessages.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,4 +12,6 @@ public interface RemediationMessages {
1212
/** Multiple calls found at the given location and that may cause confusion */
1313
String multipleCallsFound =
1414
"Multiple calls found at the given location and that may cause confusion";
15+
16+
String ambiguousCodeShape = "Ambiguous code shape";
1517
}

‎framework/codemodder-base/src/main/java/io/codemodder/remediation/xss/DefaultXSSRemediator.java‎

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
1+
package io.codemodder.remediation.xss;
2+
3+
import static java.util.stream.Collectors.groupingBy;
4+
5+
import com.github.javaparser.ast.CompilationUnit;
6+
import io.codemodder.CodemodChange;
7+
import io.codemodder.CodemodFileScanningResult;
8+
import io.codemodder.DependencyGAV;
9+
import io.codemodder.codetf.DetectorRule;
10+
import io.codemodder.codetf.FixedFinding;
11+
import io.codemodder.codetf.UnfixedFinding;
12+
import java.nio.file.Path;
13+
import java.util.ArrayList;
14+
import java.util.List;
15+
import java.util.Map;
16+
import java.util.function.Function;
17+
18+
final class DefaultXSSRemediator implements XSSRemediator {
19+
20+
private final List<XSSCodeShapeFixer> javaFixers;
21+
22+
DefaultXSSRemediator() {
23+
this.javaFixers = List.of(new PrintingMethodFixer(), new NakedVariableReturnFixer());
24+
}
25+
26+
@Override
27+
public <T> CodemodFileScanningResult remediateJava(
28+
final CompilationUnit cu,
29+
final String path,
30+
final DetectorRule detectorRule,
31+
final List<T> issuesForFile,
32+
final Function<T, String> getKey,
33+
final Function<T, Integer> getLine,
34+
final Function<T, Integer> getColumn) {
35+
36+
List<XSSFixGroup<T>> fixGroups = createFixGroups(issuesForFile, getLine, getColumn);
37+
38+
List<UnfixedFinding> unfixedFindings = new ArrayList<>();
39+
List<CodemodChange> changes = new ArrayList<>();
40+
41+
for (XSSFixGroup<T> fixGroup : fixGroups) {
42+
boolean foundResponsibleFixer = false;
43+
for (XSSCodeShapeFixer javaFixer : javaFixers) {
44+
XSSCodeShapeFixResult fixResult =
45+
javaFixer.fixCodeShape(
46+
cu, path, detectorRule, fixGroup.issues(), getKey, getLine, getColumn);
47+
if (fixResult.isResponsibleFixer()) {
48+
foundResponsibleFixer = true;
49+
if (fixResult.isFixed()) {
50+
List<FixedFinding> fixes =
51+
fixGroup.issues().stream()
52+
.map(fix -> new FixedFinding(getKey.apply(fix), detectorRule))
53+
.toList();
54+
changes.add(
55+
CodemodChange.from(
56+
fixResult.line(), List.of(DependencyGAV.OWASP_XSS_JAVA_ENCODER), fixes));
57+
} else {
58+
List<UnfixedFinding> unfixed =
59+
fixGroup.issues().stream()
60+
.map(
61+
fix ->
62+
new UnfixedFinding(
63+
getKey.apply(fix),
64+
detectorRule,
65+
path,
66+
fixResult.line(),
67+
fixResult.reasonNotFixed()))
68+
.toList();
69+
unfixedFindings.addAll(unfixed);
70+
}
71+
// if this was the responsible fixer, no need to continue
72+
break;
73+
}
74+
}
75+
if (!foundResponsibleFixer) {
76+
unfixedFindings.addAll(
77+
fixGroup.issues().stream()
78+
.map(
79+
fix ->
80+
new UnfixedFinding(
81+
getKey.apply(fix),
82+
detectorRule,
83+
path,
84+
getLine.apply(fix),
85+
"Couldn't fix that shape of code"))
86+
.toList());
87+
}
88+
}
89+
return CodemodFileScanningResult.from(changes, unfixedFindings);
90+
}
91+
92+
/** Split the issues into fix groups that all have the same location. */
93+
private <T> List<XSSFixGroup<T>> createFixGroups(
94+
final List<T> issuesForFile,
95+
final Function<T, Integer> getLine,
96+
final Function<T, Integer> getColumn) {
97+
List<XSSFixGroup<T>> fixGroups = new ArrayList<>();
98+
99+
Map<Integer, List<T>> fixesPerLine = issuesForFile.stream().collect(groupingBy(getLine));
100+
101+
// now further separate the fixes by column if it's available
102+
for (Map.Entry<Integer, List<T>> entry : fixesPerLine.entrySet()) {
103+
Map<Integer, List<T>> fixesPerColumn =
104+
entry.getValue().stream().collect(groupingBy(getColumn));
105+
for (List<T> columnFixes : fixesPerColumn.values()) {
106+
fixGroups.add(new XSSFixGroup<>(columnFixes));
107+
}
108+
}
109+
110+
return List.copyOf(fixGroups);
111+
}
112+
113+
@Override
114+
public <T> CodemodFileScanningResult remediateJSP(
115+
final Path filePath,
116+
final String path,
117+
final DetectorRule detectorRule,
118+
final List<T> issuesForFile,
119+
final Function<T, String> getKey,
120+
final Function<T, Integer> getLine,
121+
final Function<T, Integer> getColumn) {
122+
return CodemodFileScanningResult.none();
123+
}
124+
}

‎framework/codemodder-base/src/main/java/io/codemodder/remediation/xss/NakedVariableReturnFixer.java‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
package io.codemodder.remediation.xss;
2+
3+
import static io.codemodder.javaparser.JavaParserTransformer.wrap;
4+
5+
import com.github.javaparser.ast.CompilationUnit;
6+
import com.github.javaparser.ast.stmt.ReturnStmt;
7+
import io.codemodder.codetf.DetectorRule;
8+
import io.codemodder.remediation.RemediationMessages;
9+
import java.util.List;
10+
import java.util.function.Function;
11+
12+
/**
13+
* Fixes a return statement that returns a {@link String} expression.
14+
*
15+
* <pre>{@code
16+
* return foo; // should become return Encode.forHtml(foo)
17+
* }</pre>
18+
*/
19+
final class NakedVariableReturnFixer implements XSSCodeShapeFixer {
20+
21+
@Override
22+
public <T> XSSCodeShapeFixResult fixCodeShape(
23+
final CompilationUnit cu,
24+
final String path,
25+
final DetectorRule detectorRule,
26+
final List<T> issues,
27+
final Function<T, String> getKey,
28+
final Function<T, Integer> getLine,
29+
final Function<T, Integer> getColumn) {
30+
int line = getLine.apply(issues.get(0));
31+
Integer column = getColumn.apply(issues.get(0));
32+
33+
List<ReturnStmt> matchingStatements =
34+
cu.findAll(ReturnStmt.class).stream()
35+
.filter(rs -> rs.getRange().isPresent())
36+
.filter(rs -> rs.getRange().get().begin.line == line)
37+
.filter(rs -> column == null || rs.getRange().get().begin.column == column)
38+
.filter(rs -> rs.getExpression().isPresent())
39+
.filter(rs -> rs.getExpression().get().isNameExpr())
40+
.toList();
41+
42+
if (matchingStatements.isEmpty()) {
43+
return new XSSCodeShapeFixResult(false, false, null, line);
44+
} else if (matchingStatements.size() > 1) {
45+
return new XSSCodeShapeFixResult(true, false, RemediationMessages.multipleCallsFound, line);
46+
}
47+
48+
ReturnStmt nakedReturn = matchingStatements.get(0);
49+
wrap(nakedReturn.getExpression().get())
50+
.withStaticMethod("org.owasp.encoder.Encode", "forHtml", false);
51+
52+
return new XSSCodeShapeFixResult(true, true, null, line);
53+
}
54+
}

‎framework/codemodder-base/src/main/java/io/codemodder/remediation/xss/PrintingMethodFixer.java‎

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
package io.codemodder.remediation.xss;
2+
3+
import static io.codemodder.javaparser.JavaParserTransformer.wrap;
4+
5+
import com.github.javaparser.ast.CompilationUnit;
6+
import com.github.javaparser.ast.Node;
7+
import com.github.javaparser.ast.expr.Expression;
8+
import com.github.javaparser.ast.expr.MethodCallExpr;
9+
import com.github.javaparser.ast.expr.NameExpr;
10+
import com.github.javaparser.ast.nodeTypes.NodeWithType;
11+
import io.codemodder.ast.ASTs;
12+
import io.codemodder.codetf.DetectorRule;
13+
import io.codemodder.remediation.RemediationMessages;
14+
import java.util.List;
15+
import java.util.Optional;
16+
import java.util.Set;
17+
import java.util.function.Function;
18+
19+
/**
20+
* Fixes a method invocation that prints {@link String} expression.
21+
*
22+
* <pre>{@code
23+
* out.println(foo); // should become return out.println(Encode.forHtml(foo));
24+
* }</pre>
25+
*/
26+
final class PrintingMethodFixer implements XSSCodeShapeFixer {
27+
28+
private static final Set<String> writingMethodNames = Set.of("print", "println", "write");
29+
30+
@Override
31+
public <T> XSSCodeShapeFixResult fixCodeShape(
32+
final CompilationUnit cu,
33+
final String path,
34+
final DetectorRule detectorRule,
35+
final List<T> issues,
36+
final Function<T, String> getKey,
37+
final Function<T, Integer> getLine,
38+
final Function<T, Integer> getColumn) {
39+
int line = getLine.apply(issues.get(0));
40+
Integer column = getColumn.apply(issues.get(0));
41+
42+
List<MethodCallExpr> writingMethodCalls =
43+
cu.findAll(MethodCallExpr.class).stream()
44+
.filter(mce -> writingMethodNames.contains(mce.getNameAsString()))
45+
.filter(mce -> mce.getArguments().size() == 1)
46+
.filter(mce -> mce.getRange().isPresent())
47+
.filter(mce -> mce.getRange().get().begin.line == line)
48+
.filter(mce -> column == null || mce.getRange().get().begin.column == column)
49+
.filter(
50+
mce -> {
51+
Expression arg = mce.getArgument(0);
52+
if (arg instanceof NameExpr nameExpr) {
53+
Optional<Node> source =
54+
ASTs.findNonCallableSimpleNameSource(nameExpr.getName());
55+
if (source.isPresent()
56+
&& source.get() instanceof NodeWithType<?, ?> typedNode) {
57+
String typeAsString = typedNode.getTypeAsString();
58+
return "String".equals(typeAsString)
59+
|| "java.lang.String".equals(typeAsString);
60+
}
61+
}
62+
return false;
63+
})
64+
.toList();
65+
66+
if (writingMethodCalls.isEmpty()) {
67+
return new XSSCodeShapeFixResult(false, false, null, line);
68+
} else if (writingMethodCalls.size() > 1) {
69+
return new XSSCodeShapeFixResult(true, false, RemediationMessages.multipleCallsFound, line);
70+
}
71+
72+
MethodCallExpr call = writingMethodCalls.get(0);
73+
wrap(call.getArgument(0)).withStaticMethod("org.owasp.encoder.Encode", "forHtml", false);
74+
return new XSSCodeShapeFixResult(true, true, null, line);
75+
}
76+
}

‎framework/codemodder-base/src/main/java/io/codemodder/remediation/xss/XSSCodeShapeFixResult.java‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
package io.codemodder.remediation.xss;
2+
3+
/** The results of a fix attempt. */
4+
record XSSCodeShapeFixResult(
5+
boolean isResponsibleFixer, boolean isFixed, String reasonNotFixed, int line) {
6+
XSSCodeShapeFixResult {
7+
if (!isResponsibleFixer && isFixed) {
8+
throw new IllegalArgumentException("Must be responsible fixer if fixed");
9+
}
10+
if (isFixed && reasonNotFixed != null) {
11+
throw new IllegalArgumentException("Cannot be fixed and have a reason not fixed");
12+
}
13+
if (isResponsibleFixer && !isFixed && reasonNotFixed == null) {
14+
throw new IllegalArgumentException("Must have a reason not fixed if not fixed");
15+
}
16+
if (isFixed && line < 0) {
17+
throw new IllegalArgumentException("Line must be positive");
18+
}
19+
}
20+
}

‎framework/codemodder-base/src/main/java/io/codemodder/remediation/xss/XSSCodeShapeFixer.java‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
package io.codemodder.remediation.xss;
2+
3+
import com.github.javaparser.ast.CompilationUnit;
4+
import io.codemodder.codetf.DetectorRule;
5+
import java.util.List;
6+
import java.util.function.Function;
7+
8+
/** A function that can fix XSS that has a particular code shape. */
9+
interface XSSCodeShapeFixer {
10+
11+
/**
12+
* Find the shape of code at this location, and fix it if it's the expected shape.
13+
*
14+
* @param cu the compilation unit to fix
15+
* @param path the path of the file being fixed
16+
* @param detectorRule the detector rule that found the issue
17+
* @param fixGroup the group of issues to fix
18+
* @param getKey a function to get the key of an issue
19+
* @param getLine a function to get the line of an issue
20+
* @param getColumn a function to get the column of an issue
21+
* @return the result of the fix
22+
*/
23+
<T> XSSCodeShapeFixResult fixCodeShape(
24+
CompilationUnit cu,
25+
String path,
26+
DetectorRule detectorRule,
27+
List<T> fixGroup,
28+
Function<T, String> getKey,
29+
Function<T, Integer> getLine,
30+
Function<T, Integer> getColumn);
31+
}

‎framework/codemodder-base/src/main/java/io/codemodder/remediation/xss/XSSFixGroup.java‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
package io.codemodder.remediation.xss;
2+
3+
import java.util.List;
4+
import java.util.Objects;
5+
6+
/** A collection of issues that are all fixable at the same line (and column, if available). */
7+
record XSSFixGroup<T>(List<T> issues) {
8+
XSSFixGroup {
9+
Objects.requireNonNull(issues, "issues");
10+
if (issues.isEmpty()) {
11+
throw new IllegalArgumentException("Must have at least one issue");
12+
}
13+
}
14+
}

‎framework/codemodder-base/src/main/java/io/codemodder/remediation/xss/XSSRemediator.java‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
package io.codemodder.remediation.xss;
2+
3+
import com.github.javaparser.ast.CompilationUnit;
4+
import io.codemodder.CodemodFileScanningResult;
5+
import io.codemodder.codetf.DetectorRule;
6+
import java.nio.file.Path;
7+
import java.util.List;
8+
import java.util.function.Function;
9+
10+
/** Remediates header injection vulnerabilities. */
11+
public interface XSSRemediator {
12+
13+
/** Remediate all header injection vulnerabilities in the given compilation unit. */
14+
<T> CodemodFileScanningResult remediateJava(
15+
CompilationUnit cu,
16+
String path,
17+
DetectorRule detectorRule,
18+
List<T> issuesForFile,
19+
Function<T, String> getKey,
20+
Function<T, Integer> getLine,
21+
Function<T, Integer> getColumn);
22+
23+
<T> CodemodFileScanningResult remediateJSP(
24+
Path filePath,
25+
String relativePath,
26+
DetectorRule detectorRule,
27+
List<T> issuesForFile,
28+
Function<T, String> getKey,
29+
Function<T, Integer> getLine,
30+
Function<T, Integer> getColumn);
31+
32+
/** The default header injection remediation strategy. */
33+
XSSRemediator DEFAULT = new DefaultXSSRemediator();
34+
}

0 commit comments

Comments
 (0)