Skip to content

Commit

Permalink
Added support for BinaryExpr findings
Browse files Browse the repository at this point in the history
  • Loading branch information
andrecsilva committed Oct 15, 2024
1 parent e667d3d commit 5a4ccec
Showing 1 changed file with 62 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2,30 +2,83 @@

import com.github.javaparser.ast.CompilationUnit;
import com.github.javaparser.ast.Node;
import com.github.javaparser.ast.expr.BinaryExpr;
import com.github.javaparser.ast.expr.Expression;
import com.github.javaparser.ast.expr.MethodCallExpr;
import io.codemodder.Either;
import io.codemodder.ast.ASTs;
import io.codemodder.ast.LocalVariableDeclaration;
import io.codemodder.remediation.RemediationStrategy;
import io.codemodder.remediation.SuccessOrReason;
import java.util.Optional;

/** Composes several transformations related to SQL injections. */
public final class SQLInjectionFixComposer implements RemediationStrategy {

SQLInjectionFixComposer() {}
public SQLInjectionFixComposer() {}

/**
* Given a node, check if it is a {@link MethodCallExpr} related to executing JDBC API SQL queries
* (i.e. prepareStatement(), executeQuery(), etc.), parameterize data injections or add a
* validation step for structural injections.
* Checks if the given binaryExpr ends up as an argument for a SQL execute method that can be
* fixed.
*
* @param binaryExpr
* @return An Optional containing the execute call if successful
*/
private Optional<MethodCallExpr> flowsIntoExecuteCall(final BinaryExpr binaryExpr) {
// Is argument of a method call
var maybeDirectArgumentOfCall =
ASTs.isArgumentOfMethodCall(binaryExpr).filter(SQLInjectionFixComposer::match);

// or it is initialization of a variable that flows into an execute call
return maybeDirectArgumentOfCall.or(() -> isIndirectArgumentOfCall(binaryExpr));
}

/**
* Test if the expr is initialized into a variable that flows into a call
*
* @param expr
* @return
*/
private Optional<MethodCallExpr> isIndirectArgumentOfCall(final Expression expr) {
return ASTs.isInitExpr(expr)
.flatMap(LocalVariableDeclaration::fromVariableDeclarator)
.flatMap(
lvd ->
lvd.findAllReferences()
.flatMap(ne -> ASTs.isArgumentOfMethodCall(ne).stream())
.filter(SQLInjectionFixComposer::match)
.findFirst());
}

/**
* Given a node, checks if it is a {@link MethodCallExpr} related to executing JDBC API SQL
* queries (i.e. prepareStatement(), executeQuery(), etc.), or a {@link BinaryExpr} that flows
* into one, parameterize data injections or add a validation step for structural injections.
*/
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");
// Is a binary expr or method call expr?
Optional<Either<MethodCallExpr, BinaryExpr>> morb;
if (node instanceof MethodCallExpr m) {
morb = Optional.of(Either.left(m));
} else if (node instanceof BinaryExpr b) {
morb = Optional.of(Either.right(b));
} else {
morb = Optional.empty();
}
if (morb.isEmpty()) {
return SuccessOrReason.reason("Neither a binary expression or method call");
}
// If binary expr, try to find if it flows into a function as argument
// map the left into optional for type consistency
Optional<MethodCallExpr> maybeCall =
morb.flatMap(e -> e.ifLeftOrElseGet(Optional::of, this::flowsIntoExecuteCall));
if (maybeCall.isEmpty()) {
return SuccessOrReason.reason(
"Could not find any execute call that the binary expr flows into");
}

MethodCallExpr methodCallExpr = maybeMethodCall.get();
MethodCallExpr methodCallExpr = maybeCall.get();

// First, check if any data injection fixes apply
var maybeFixed = new SQLParameterizer(methodCallExpr, cu).checkAndFix();
Expand Down

0 comments on commit 5a4ccec

Please sign in to comment.