Adds new transformation for SQL injection/parameterization codemods (#463)

andrecsilva · web-flow · commit 882f436ef956 · 2024-11-08T15:24:43.000-03:00
Also adjusts the add statements method to work around some
`LexicalPreservingPrinter` issues. This fixed some spacing, indentation
issues in some tests.
diff --git a/core-codemods/src/test/java/io/codemodder/codemods/MoveSwitchDefaultCaseLastCodemodTest.java b/core-codemods/src/test/java/io/codemodder/codemods/MoveSwitchDefaultCaseLastCodemodTest.java
@@ -57,9 +57,9 @@ void foo() {
                           break;
                         default:
                           break;
-                case 0:
-                    break;
-                     }
+                        case 0:
+                            break;
+                        }
                   }
                 }
                 """;
diff --git a/core-codemods/src/test/java/io/codemodder/codemods/SQLParameterizerCodemodTest.java b/core-codemods/src/test/java/io/codemodder/codemods/SQLParameterizerCodemodTest.java
@@ -2,9 +2,21 @@
 
 import io.codemodder.testutils.CodemodTestMixin;
 import io.codemodder.testutils.Metadata;
+import org.junit.jupiter.api.Nested;
 
-@Metadata(
-    codemodType = SQLParameterizerCodemod.class,
-    testResourceDir = "sql-parameterizer",
-    dependencies = {})
-final class SQLParameterizerCodemodTest implements CodemodTestMixin {}
+final class SQLParameterizerCodemodTest {
+
+  @Nested
+  @Metadata(
+      codemodType = SQLParameterizerCodemod.class,
+      testResourceDir = "sql-parameterizer/defaultTransformation",
+      dependencies = {})
+  class DefaultTransformationTest implements CodemodTestMixin {}
+
+  @Nested
+  @Metadata(
+      codemodType = SQLParameterizerCodemod.class,
+      testResourceDir = "sql-parameterizer/hijackTransformation",
+      dependencies = {})
+  class HijackTransformationTest implements CodemodTestMixin {}
+}
diff --git a/core-codemods/src/test/resources/defectdojo-sql-injection/SqlInjectionChallenge/SqlInjectionChallenge.java.after b/core-codemods/src/test/resources/defectdojo-sql-injection/SqlInjectionChallenge/SqlInjectionChallenge.java.after
@@ -68,6 +68,7 @@ public class SqlInjectionChallenge extends AssignmentEndpoint {
             "select userid from sql_challenge_users where userid = ?";
         PreparedStatement statement = connection.prepareStatement(checkUserQuery);
         statement.setString(1, username_reg);
+
         ResultSet resultSet = statement.execute();
         if (resultSet.next()) {
           if (username_reg.contains("tom'")) {
@@ -84,7 +85,6 @@ public class SqlInjectionChallenge extends AssignmentEndpoint {
           preparedStatement.execute();
           attackResult = success(this).feedback("user.created").feedbackArgs(username_reg).build();
         }
-
       } catch (SQLException e) {
         attackResult = failed(this).output("Something went wrong").build();
       }
diff --git a/core-codemods/src/test/resources/defectdojo-sql-injection/SqlInjectionLesson8/SqlInjectionLesson8.java.after b/core-codemods/src/test/resources/defectdojo-sql-injection/SqlInjectionLesson8/SqlInjectionLesson8.java.after
@@ -70,9 +70,10 @@ public class SqlInjectionLesson8 extends AssignmentEndpoint {
       try {
         PreparedStatement statement =
             connection.prepareStatement(
-query,                 ResultSet.TYPE_SCROLL_INSENSITIVE, ResultSet.CONCUR_UPDATABLE);
+            query,     ResultSet.TYPE_SCROLL_INSENSITIVE, ResultSet.CONCUR_UPDATABLE);
         log(connection, query);
         statement.setString(1, name);
+
         statement.setString(2, auth_tan);
         ResultSet results = statement.execute();
         if (results.getStatement() != null) {
@@ -98,7 +99,6 @@ query,                 ResultSet.TYPE_SCROLL_INSENSITIVE, ResultSet.CONCUR_UPDAT
         } else {
           return failed(this).build();
         }
-
       } catch (SQLException e) {
         return failed(this)
             .output("<br><span class='feedback-negative'>" + e.getMessage() + "</span>")
@@ -156,7 +156,7 @@ query,                 ResultSet.TYPE_SCROLL_INSENSITIVE, ResultSet.CONCUR_UPDAT
       statement.setString(1, sdf.format(cal.getTime()));
       statement.setString(2, action);
       statement.execute();
-        } catch (SQLException e) {
+    } catch (SQLException e) {
       System.err.println(e.getMessage());
     }
   }
diff --git a/core-codemods/src/test/resources/jexl-expression-injection/Test.java.after b/core-codemods/src/test/resources/jexl-expression-injection/Test.java.after
@@ -27,7 +27,6 @@ public final class Test {
       JexlExpression expression = jexl.createExpression(input);
       JexlContext context = new MapContext();
       expression.evaluate(context);
-
     }
   }
 
@@ -42,7 +41,6 @@ public final class Test {
           sandbox.block(cls);
       }
       new JexlBuilder().sandbox(sandbox).create().createExpression(input).evaluate(context);
-
     }
   }
 
diff --git a/core-codemods/src/test/resources/move-switch-default-last/Test.java.after b/core-codemods/src/test/resources/move-switch-default-last/Test.java.after
@@ -11,8 +11,8 @@ final class Test {
            case "bar":
              System.out.println("bar");
              break;
-default:
-             System.out.println("default");       }
+           default:
+             System.out.println("default");}
        System.out.println("bar");
    }
 
diff --git a/core-codemods/src/test/resources/semgrep-sql-injection-formatted-sql-string/SqlInjectionLesson5a.java.after b/core-codemods/src/test/resources/semgrep-sql-injection-formatted-sql-string/SqlInjectionLesson5a.java.after
@@ -64,8 +64,9 @@ public class SqlInjectionLesson5a extends AssignmentEndpoint {
           "SELECT * FROM user_data WHERE first_name = 'John' and last_name = ?";
       try (PreparedStatement statement =
           connection.prepareStatement(
-query,               ResultSet.TYPE_SCROLL_INSENSITIVE, ResultSet.CONCUR_UPDATABLE)) {
+          query,     ResultSet.TYPE_SCROLL_INSENSITIVE, ResultSet.CONCUR_UPDATABLE)) {
         statement.setString(1, accountName);
+
         ResultSet results = statement.execute();
         if ((results != null) && (results.first())) {
           ResultSetMetaData resultsMetaData = results.getMetaData();
@@ -90,7 +91,6 @@ query,               ResultSet.TYPE_SCROLL_INSENSITIVE, ResultSet.CONCUR_UPDATAB
               .output("Your query was: " + query)
               .build();
         }
-
       } catch (SQLException sqle) {
         return failed(this).output(sqle.getMessage() + "<br> Your query was: " + query).build();
       }
@@ -135,4 +135,4 @@ query,               ResultSet.TYPE_SCROLL_INSENSITIVE, ResultSet.CONCUR_UPDATAB
     t.append("</p>");
     return (t.toString());
   }
-}
+}
diff --git a/core-codemods/src/test/resources/semgrep-sql-injection/SqlInjectionLesson8.java.after b/core-codemods/src/test/resources/semgrep-sql-injection/SqlInjectionLesson8.java.after
@@ -70,9 +70,10 @@ public class SqlInjectionLesson8 extends AssignmentEndpoint {
       try {
         PreparedStatement statement =
             connection.prepareStatement(
-query,                 ResultSet.TYPE_SCROLL_INSENSITIVE, ResultSet.CONCUR_UPDATABLE);
+            query,     ResultSet.TYPE_SCROLL_INSENSITIVE, ResultSet.CONCUR_UPDATABLE);
         log(connection, query);
         statement.setString(1, name);
+
         statement.setString(2, auth_tan);
         ResultSet results = statement.execute();
         if (results.getStatement() != null) {
@@ -98,7 +99,6 @@ query,                 ResultSet.TYPE_SCROLL_INSENSITIVE, ResultSet.CONCUR_UPDAT
         } else {
           return failed(this).build();
         }
-
       } catch (SQLException e) {
         return failed(this)
             .output("<br><span class='feedback-negative'>" + e.getMessage() + "</span>")
@@ -156,7 +156,7 @@ query,                 ResultSet.TYPE_SCROLL_INSENSITIVE, ResultSet.CONCUR_UPDAT
       statement.setString(1, sdf.format(cal.getTime()));
       statement.setString(2, action);
       statement.execute();
-        } catch (SQLException e) {
+    } catch (SQLException e) {
       System.err.println(e.getMessage());
     }
   }
diff --git a/core-codemods/src/test/resources/sonar-sql-injection-s2077/supported/SqlInjectionChallenge.java.after b/core-codemods/src/test/resources/sonar-sql-injection-s2077/supported/SqlInjectionChallenge.java.after
@@ -68,6 +68,7 @@ public class SqlInjectionChallenge extends AssignmentEndpoint {
             "select userid from sql_challenge_users where userid = ?";
         PreparedStatement statement = connection.prepareStatement(checkUserQuery);
         statement.setString(1, username_reg);
+
         ResultSet resultSet = statement.execute();
         if (resultSet.next()) {
           if (username_reg.contains("tom'")) {
@@ -84,7 +85,6 @@ public class SqlInjectionChallenge extends AssignmentEndpoint {
           preparedStatement.execute();
           attackResult = success(this).feedback("user.created").feedbackArgs(username_reg).build();
         }
-
       } catch (SQLException e) {
         attackResult = failed(this).output("Something went wrong").build();
       }
diff --git a/core-codemods/src/test/resources/sql-parameterizer/defaultTransformation/Test.java.after b/core-codemods/src/test/resources/sql-parameterizer/defaultTransformation/Test.java.after
@@ -23,7 +23,7 @@ public final class Test {
     stmt.setString(1, input);
     var rs = stmt.execute();
     return rs;
-    }
+  }
 
   public ResultSet nameConflict(String input) throws SQLException {
     int stmt = 0;
@@ -33,7 +33,7 @@ public final class Test {
     ResultSet rs = statement.execute();
     stmt++;
     return rs;
-    }
+  }
 
   public ResultSet doubleNameConflict(String input) throws SQLException {
     int stmt = 0;
@@ -44,7 +44,7 @@ public final class Test {
     ResultSet rs = stmt1.execute();
     stmt = stmt + statement;
     return rs;
-    }
+  }
 
   public ResultSet tryResource(String input) throws SQLException {
     String sql = "SELECT * FROM USERS WHERE USER = ?";
@@ -62,7 +62,7 @@ public final class Test {
     stmt.setString(1, "user_" + input + "_name");
     stmt.setString(2, input2);
     return stmt.execute();
-    }
+  }
 
   public ResultSet referencesAfterExecute(String input) throws SQLException {
     String sql = "SELECT * FROM USERS WHERE USER = ?";
diff --git a/core-codemods/src/test/resources/sql-parameterizer/defaultTransformation/Test.java.before b/core-codemods/src/test/resources/sql-parameterizer/defaultTransformation/Test.java.before
diff --git a/core-codemods/src/test/resources/sql-parameterizer/hijackTransformation/Test.java.after b/core-codemods/src/test/resources/sql-parameterizer/hijackTransformation/Test.java.after
@@ -0,0 +1,33 @@
+package com.acme.testcode;
+
+import java.sql.Connection;
+import java.sql.PreparedStatement;
+import java.sql.ResultSet;
+import java.sql.SQLException;
+import java.sql.Statement;
+
+public final class Test {
+
+  private Connection conn;
+
+  public void queryAfterDeclaration() throws SQLException {
+	Statement stmt;
+	String query2 = "SELECT * FROM users WHERE username = ?";
+	PreparedStatement statement = conn.prepareStatement(query2);
+	statement.setString(1, request.getParameter("username"));
+	ResultSet rs2 = statement.execute();
+	stmt = statement;
+	while (rs2.next()) {
+		System.out.println("User: " + rs2.getString("username"));
+	}
+	String query3 = "SELECT * FROM users WHERE email = ?";
+	stmt.close();
+	PreparedStatement stmt1 = conn.prepareStatement(query3);
+	stmt1.setString(1, request.getParameter("email"));
+	ResultSet rs3 = stmt1.execute();
+	stmt = stmt1;
+	while (rs3.next()) {
+		System.out.println("User: " + rs3.getString("username"));
+	}
+  }
+}
diff --git a/core-codemods/src/test/resources/sql-parameterizer/hijackTransformation/Test.java.before b/core-codemods/src/test/resources/sql-parameterizer/hijackTransformation/Test.java.before
@@ -0,0 +1,27 @@
+package com.acme.testcode;
+
+import java.sql.Connection;
+import java.sql.ResultSet;
+import java.sql.SQLException;
+import java.sql.Statement;
+
+public final class Test {
+
+  private Connection conn;
+
+  public void queryAfterDeclaration() throws SQLException {
+	Statement stmt = conn.createStatement();
+	String username = request.getParameter("username");
+	String query2 = "SELECT * FROM users WHERE username = '" + username + "'";
+	ResultSet rs2 = stmt.executeQuery(query2);
+	while (rs2.next()) {
+		System.out.println("User: " + rs2.getString("username"));
+	}
+	String email = request.getParameter("email");
+	String query3 = "SELECT * FROM users WHERE email = '" + email + "'";
+	ResultSet rs3 = stmt.executeQuery(query3);
+	while (rs3.next()) {
+		System.out.println("User: " + rs3.getString("username"));
+	}
+  }
+}
diff --git a/framework/codemodder-base/src/main/java/io/codemodder/ast/ASTTransforms.java b/framework/codemodder-base/src/main/java/io/codemodder/ast/ASTTransforms.java
@@ -26,7 +26,6 @@
 import com.github.javaparser.resolution.types.ResolvedType;
 import java.util.ArrayList;
 import java.util.Optional;
-import java.util.stream.IntStream;
 
 public final class ASTTransforms {
   /** Add an import in alphabetical order. */
@@ -87,26 +86,21 @@ public static void addStaticImportIfMissing(final CompilationUnit cu, final Stri
    */
   public static <N extends Node> void addStatementAt(
       final NodeWithStatements<N> node, final Statement stmt, final int index) {
+
+    var oldStatements = node.getStatements();
     var newStatements = new ArrayList<Statement>();
     int i = 0;
-    for (var s : node.getStatements()) {
+    for (var s : oldStatements) {
       if (i == index) {
         newStatements.add(stmt);
       }
       newStatements.add(s);
       i++;
     }
-
-    // rebuilds the whole statements list as to preserve proper children order.
-
-    // workaround for maintaining indent, removes all but the first
-    IntStream.range(0, node.getStatements().size() - 1)
-        .forEach(j -> node.getStatements().removeLast());
-    // replace the first with the new statement if needed
-    if (index == 0) {
-      node.getStatements().get(0).replace(stmt);
+    for (i = index; i < oldStatements.size(); i++) {
+      node.setStatement(i, newStatements.get(i));
     }
-    newStatements.stream().skip(1).forEach(node.getStatements()::add);
+    node.addStatement(newStatements.get(newStatements.size() - 1));
   }
 
   /**
@@ -291,7 +285,13 @@ public static Expression removeEmptyStringConcatenation(final BinaryExpr binexp)
   public static void removeEmptyStringConcatenation(Node subtree) {
     subtree
         .findAll(BinaryExpr.class, Node.TreeTraversal.POSTORDER)
-        .forEach(binexp -> binexp.replace(removeEmptyStringConcatenation(binexp)));
+        .forEach(
+            binexp -> {
+              var newNode = removeEmptyStringConcatenation(binexp);
+              if (newNode != binexp) {
+                binexp.replace(newNode.clone());
+              }
+            });
   }
 
   /** Removes unused variables. */
diff --git a/framework/codemodder-base/src/main/java/io/codemodder/ast/ASTs.java b/framework/codemodder-base/src/main/java/io/codemodder/ast/ASTs.java
@@ -186,11 +186,11 @@ public static Optional<MethodCallExpr> isScopeInMethodCall(final Expression expr
    *
    * @return A tuple with the above pattern in order sans the {@link SimpleName}.
    */
-  public static Optional<PatternExpr> isPatternExprDeclarationOf(
+  public static Optional<TypePatternExpr> isPatternExprDeclarationOf(
       final Node node, final String name) {
-    if (node instanceof PatternExpr) {
-      var pexpr = (PatternExpr) node;
-      if (pexpr.getNameAsString().equals(name)) return Optional.of(pexpr);
+    if (node instanceof TypePatternExpr) {
+      var pexpr = (TypePatternExpr) node;
+      if (pexpr.getName().asString().equals(name)) return Optional.of(pexpr);
     }
     return Optional.empty();
   }
diff --git a/framework/codemodder-base/src/main/java/io/codemodder/remediation/sqlinjection/SQLParameterizer.java b/framework/codemodder-base/src/main/java/io/codemodder/remediation/sqlinjection/SQLParameterizer.java
diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml

Original file line number	Diff line number	Diff line change
`@@ -57,9 +57,9 @@ void foo() {`
`57`	`57`	`break;`
`58`	`58`	`default:`
`59`	`59`	`break;`
`60`		`- case 0:`
`61`		`- break;`
`62`		`- }`
	`60`	`+ case 0:`
	`61`	`+ break;`
	`62`	`+ }`
`63`	`63`	`}`
`64`	`64`	`}`
`65`	`65`	`""";`
Original file line number	Diff line number	Diff line change
`@@ -27,7 +27,6 @@ public final class Test {`
`27`	`27`	`JexlExpression expression = jexl.createExpression(input);`
`28`	`28`	`JexlContext context = new MapContext();`
`29`	`29`	`expression.evaluate(context);`
`30`		`-`
`31`	`30`	`}`
`32`	`31`	`}`
`33`	`32`
`@@ -42,7 +41,6 @@ public final class Test {`
`42`	`41`	`sandbox.block(cls);`
`43`	`42`	`}`
`44`	`43`	`new JexlBuilder().sandbox(sandbox).create().createExpression(input).evaluate(context);`
`45`		`-`
`46`	`44`	`}`
`47`	`45`	`}`
`48`	`46`