Skip to content

Commit 560dbfc

Browse files
nahsranahsra
authored
Create JNDI alternate fix (#401)
Create another strategy for JNDI that swaps out the sink for a hardened version.
1 parent 1bff988 commit 560dbfc

File tree

7 files changed

+226
-76
lines changed

7 files changed

+226
-76
lines changed

‎framework/codemodder-base/src/main/java/io/codemodder/DependencyGAV.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,7 @@ static DependencyGAV createDefault(
160160
group, artifact, version, justification, license, repositoryUrl, noTransitiveDependencies);
161161
}
162162

163-
String JAVA_SECURITY_TOOLKIT_VERSION = "1.1.3";
163+
String JAVA_SECURITY_TOOLKIT_VERSION = "1.2.0";
164164
String JAVA_SECURITY_TOOLKIT_GAV =
165165
"io.github.pixee:java-security-toolkit:" + JAVA_SECURITY_TOOLKIT_VERSION;
166166

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

Lines changed: 12 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,5 @@
11
package io.codemodder.remediation.jndiinjection;
22

3-
import static io.codemodder.ast.ASTTransforms.addImportIfMissing;
4-
5-
import com.github.javaparser.StaticJavaParser;
63
import com.github.javaparser.ast.CompilationUnit;
74
import com.github.javaparser.ast.Node;
85
import com.github.javaparser.ast.body.ClassOrInterfaceDeclaration;
@@ -13,6 +10,7 @@
1310
import com.github.javaparser.ast.stmt.Statement;
1411
import io.codemodder.CodemodChange;
1512
import io.codemodder.CodemodFileScanningResult;
13+
import io.codemodder.DependencyGAV;
1614
import io.codemodder.codetf.DetectorRule;
1715
import io.codemodder.codetf.FixedFinding;
1816
import io.codemodder.codetf.UnfixedFinding;
@@ -21,29 +19,20 @@
2119
import io.codemodder.remediation.FixCandidateSearcher;
2220
import java.util.ArrayList;
2321
import java.util.List;
22+
import java.util.Objects;
2423
import java.util.Optional;
25-
import java.util.Set;
2624
import java.util.function.Function;
2725

2826
final class DefaultJNDIInjectionRemediator implements JNDIInjectionRemediator {
2927

30-
private final MethodDeclaration fixMethod;
28+
private final JNDIFixStrategy fixStrategy;
3129

3230
DefaultJNDIInjectionRemediator() {
33-
String fixMethodCode =
34-
"""
35-
private static void validateResourceName(final String name) {
36-
if (name != null) {
37-
Set<String> illegalNames = Set.of("ldap://", "rmi://", "dns://", "java:");
38-
String canonicalName = name.toLowerCase().trim();
39-
if (illegalNames.stream().anyMatch(canonicalName::startsWith)) {
40-
throw new SecurityException("Illegal JNDI resource name: " + name);
41-
}
42-
}
43-
}
44-
""";
45-
46-
this.fixMethod = StaticJavaParser.parseMethodDeclaration(fixMethodCode);
31+
this(new ReplaceLimitedLookupStrategy());
32+
}
33+
34+
DefaultJNDIInjectionRemediator(final JNDIFixStrategy fixStrategy) {
35+
this.fixStrategy = Objects.requireNonNull(fixStrategy);
4736
}
4837

4938
@Override
@@ -110,8 +99,6 @@ public <T> CodemodFileScanningResult remediateAll(
11099
// validate the shape of code around the lookup call to make sure its safe to add the call and
111100
// method
112101
NameExpr contextNameVariable = lookupCall.getArgument(0).asNameExpr();
113-
MethodCallExpr validationCall = new MethodCallExpr(null, validateResourceMethodName);
114-
validationCall.addArgument(contextNameVariable);
115102

116103
Optional<Node> lookupParentNode = lookupStatement.get().getParentNode();
117104
if (lookupParentNode.isEmpty()) {
@@ -122,7 +109,7 @@ public <T> CodemodFileScanningResult remediateAll(
122109
continue;
123110
}
124111

125-
if (!(lookupParentNode.get() instanceof BlockStmt)) {
112+
if (!(lookupParentNode.get() instanceof BlockStmt blockStmt)) {
126113
UnfixedFinding unfixedFinding =
127114
new UnfixedFinding(
128115
findingId, detectorRule, path, line, "No block statement found around lookup call");
@@ -131,32 +118,15 @@ public <T> CodemodFileScanningResult remediateAll(
131118
}
132119

133120
// add the validation call to the block statement
134-
BlockStmt blockStmt = (BlockStmt) lookupParentNode.get();
135121
int index = blockStmt.getStatements().indexOf(lookupStatement.get());
136-
blockStmt.addStatement(index, validationCall);
137-
138-
// add the validation method if it's not already present
139-
boolean alreadyHasResourceValidationCallPresent =
140-
parentClass.findAll(MethodDeclaration.class).stream()
141-
.anyMatch(
142-
md ->
143-
md.getNameAsString().equals(validateResourceMethodName)
144-
&& md.getParameters().size() == 1
145-
&& md.getParameters().get(0).getTypeAsString().equals("String"));
146-
147-
if (!alreadyHasResourceValidationCallPresent) {
148-
parentClass.addMember(fixMethod);
149-
addImportIfMissing(cu, Set.class);
150-
}
151-
152-
changes.add(CodemodChange.from(line, new FixedFinding(findingId, detectorRule)));
122+
List<DependencyGAV> deps =
123+
fixStrategy.fix(cu, parentClass, lookupCall, contextNameVariable, blockStmt, index);
124+
changes.add(CodemodChange.from(line, deps, new FixedFinding(findingId, detectorRule)));
153125
}
154126

155127
List<UnfixedFinding> allUnfixedFindings = new ArrayList<>(results.unfixableFindings());
156128
allUnfixedFindings.addAll(unfixedFindings);
157129

158130
return CodemodFileScanningResult.from(changes, allUnfixedFindings);
159131
}
160-
161-
private static final String validateResourceMethodName = "validateResourceName";
162132
}

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

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
package io.codemodder.remediation.jndiinjection;
2+
3+
import static io.codemodder.ast.ASTTransforms.addImportIfMissing;
4+
5+
import com.github.javaparser.StaticJavaParser;
6+
import com.github.javaparser.ast.CompilationUnit;
7+
import com.github.javaparser.ast.body.ClassOrInterfaceDeclaration;
8+
import com.github.javaparser.ast.body.MethodDeclaration;
9+
import com.github.javaparser.ast.expr.MethodCallExpr;
10+
import com.github.javaparser.ast.expr.NameExpr;
11+
import com.github.javaparser.ast.stmt.BlockStmt;
12+
import io.codemodder.DependencyGAV;
13+
import java.util.List;
14+
import java.util.Set;
15+
16+
/**
17+
* {@inheritDoc}
18+
*
19+
* <p>Fixes by injecting a validation method into the class and calling it before the lookup() call.
20+
*/
21+
final class InjectValidationMethodStrategy implements JNDIFixStrategy {
22+
23+
private final MethodDeclaration fixMethod;
24+
25+
InjectValidationMethodStrategy() {
26+
String fixMethodCode =
27+
"""
28+
private static void validateResourceName(final String name) {
29+
if (name != null) {
30+
Set<String> illegalNames = Set.of("ldap://", "rmi://", "dns://", "java:");
31+
String canonicalName = name.toLowerCase().trim();
32+
if (illegalNames.stream().anyMatch(canonicalName::startsWith)) {
33+
throw new SecurityException("Illegal JNDI resource name: " + name);
34+
}
35+
}
36+
}
37+
""";
38+
this.fixMethod = StaticJavaParser.parseMethodDeclaration(fixMethodCode);
39+
}
40+
41+
@Override
42+
public List<DependencyGAV> fix(
43+
final CompilationUnit cu,
44+
final ClassOrInterfaceDeclaration parentClass,
45+
final MethodCallExpr lookupCall,
46+
final NameExpr contextNameVariable,
47+
final BlockStmt blockStmt,
48+
final int index) {
49+
MethodCallExpr validationCall = new MethodCallExpr(null, validateResourceMethodName);
50+
validationCall.addArgument(contextNameVariable);
51+
blockStmt.addStatement(index, validationCall);
52+
53+
// add the validation method if it's not already present
54+
boolean alreadyHasResourceValidationCallPresent =
55+
parentClass.findAll(MethodDeclaration.class).stream()
56+
.anyMatch(
57+
md ->
58+
md.getNameAsString().equals(validateResourceMethodName)
59+
&& md.getParameters().size() == 1
60+
&& md.getParameters().get(0).getTypeAsString().equals("String"));
61+
62+
if (!alreadyHasResourceValidationCallPresent) {
63+
parentClass.addMember(fixMethod);
64+
addImportIfMissing(cu, Set.class);
65+
}
66+
67+
return List.of();
68+
}
69+
70+
private static final String validateResourceMethodName = "validateResourceName";
71+
}

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

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
package io.codemodder.remediation.jndiinjection;
2+
3+
import com.github.javaparser.ast.CompilationUnit;
4+
import com.github.javaparser.ast.body.ClassOrInterfaceDeclaration;
5+
import com.github.javaparser.ast.expr.MethodCallExpr;
6+
import com.github.javaparser.ast.expr.NameExpr;
7+
import com.github.javaparser.ast.stmt.BlockStmt;
8+
import io.codemodder.DependencyGAV;
9+
import java.util.List;
10+
11+
/** Strategy for fixing JNDI injection vulnerabilities. */
12+
interface JNDIFixStrategy {
13+
14+
/**
15+
* Fix the JNDI injection vulnerability.
16+
*
17+
* @param cu the compilation unit
18+
* @param parentClass the parent type (could be nested type)
19+
* @param lookupCall the lookup() method call
20+
* @param blockStmt the block which contains the lookup statement
21+
* @param index the index of the statement in the block
22+
* @param contextNameVariable the variable that holds the JNDI context
23+
* @return the list of dependencies that need to be added to the project
24+
*/
25+
List<DependencyGAV> fix(
26+
CompilationUnit cu,
27+
ClassOrInterfaceDeclaration parentClass,
28+
MethodCallExpr lookupCall,
29+
NameExpr contextNameVariable,
30+
BlockStmt blockStmt,
31+
int index);
32+
}

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

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
package io.codemodder.remediation.jndiinjection;
2+
3+
import static io.codemodder.javaparser.JavaParserTransformer.wrap;
4+
5+
import com.github.javaparser.ast.CompilationUnit;
6+
import com.github.javaparser.ast.body.ClassOrInterfaceDeclaration;
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.stmt.BlockStmt;
11+
import io.codemodder.DependencyGAV;
12+
import io.github.pixee.security.JNDI;
13+
import java.util.List;
14+
15+
/** Replaces the call with a limited call. */
16+
final class ReplaceLimitedLookupStrategy implements JNDIFixStrategy {
17+
18+
@Override
19+
public List<DependencyGAV> fix(
20+
final CompilationUnit cu,
21+
final ClassOrInterfaceDeclaration parentClass,
22+
final MethodCallExpr lookupCall,
23+
final NameExpr contextNameVariable,
24+
final BlockStmt blockStmt,
25+
final int index) {
26+
27+
String className = JNDI.class.getName();
28+
String methodName = "limitedContext";
29+
30+
// rather than insert all new nodes, we'll replace the ones in place
31+
// ctx.lookup(foo) -> JNDI.limitedContext(ctx).lookup(foo)
32+
33+
// this is the JNDI Context object
34+
Expression jndiContext = lookupCall.getScope().get();
35+
36+
// the new scope is the static call
37+
wrap(jndiContext).withStaticMethod(className, methodName, false);
38+
39+
return List.of(DependencyGAV.JAVA_SECURITY_TOOLKIT);
40+
}
41+
}

0 commit comments

Comments
 (0)