Add fixer for Sonar SQLi issues rather than just hotspots

core-codemods/src/main/java/io/codemodder/codemods/DefaultCodemods.java

@@ -94,7 +94,8 @@ public static List<Class<? extends CodeChanger>> asList() {
         SonarJNDIInjectionCodemod.class,
         SonarObjectDeserializationCodemod.class,
         SonarRemoveUnthrowableExceptionCodemod.class,
-        SonarSQLInjectionCodemod.class,
+        SonarSQLInjectionHotspotCodemod.class,
+        SonarSQLInjectionIssueCodemod.class,
         SonarSSRFCodemod.class,
         SonarUnsafeReflectionRemediationCodemod.class,
         SonarWeakHashingAlgorithmCodemod.class,

core-codemods/src/main/java/io/codemodder/codemods/sonar/SonarSQLInjectionCodemod.java → core-codemods/src/main/java/io/codemodder/codemods/sonar/SonarSQLInjectionHotspotCodemod.java

@@ -26,13 +26,13 @@
     reviewGuidance = ReviewGuidance.MERGE_AFTER_REVIEW,
     importance = Importance.HIGH,
     executionPriority = CodemodExecutionPriority.HIGH)
-public final class SonarSQLInjectionCodemod extends SonarRemediatingJavaParserChanger {
+public final class SonarSQLInjectionHotspotCodemod extends SonarRemediatingJavaParserChanger {
 
   private final Remediator<Hotspot> remediationStrategy;
   private final RuleHotspot hotspots;
 
   @Inject
-  public SonarSQLInjectionCodemod(
+  public SonarSQLInjectionHotspotCodemod(
       @ProvidedSonarScan(ruleId = "java:S2077") final RuleHotspot hotspots) {
     super(GenericRemediationMetadata.SQL_INJECTION.reporter(), hotspots);
     this.hotspots = Objects.requireNonNull(hotspots);

core-codemods/src/main/java/io/codemodder/codemods/sonar/SonarSQLInjectionIssueCodemod.java

@@ -0,0 +1,83 @@
+package io.codemodder.codemods.sonar;
+
+import com.github.javaparser.ast.CompilationUnit;
+import com.github.javaparser.ast.expr.Expression;
+import io.codemodder.*;
+import io.codemodder.ast.ASTs;
+import io.codemodder.codetf.DetectorRule;
+import io.codemodder.providers.sonar.ProvidedSonarScan;
+import io.codemodder.providers.sonar.RuleIssue;
+import io.codemodder.providers.sonar.SonarRemediatingJavaParserChanger;
+import io.codemodder.remediation.FixCandidateSearcher;
+import io.codemodder.remediation.GenericRemediationMetadata;
+import io.codemodder.remediation.Remediator;
+import io.codemodder.remediation.SearcherStrategyRemediator;
+import io.codemodder.remediation.sqlinjection.SQLInjectionFixComposer;
+import io.codemodder.sonar.model.Issue;
+import io.codemodder.sonar.model.SonarFinding;
+import io.codemodder.sonar.model.TextRange;
+import java.util.List;
+import java.util.Objects;
+import java.util.Optional;
+import javax.inject.Inject;
+
+@Codemod(
+    id = "sonar:java/sonar-sql-injection-s3649",
+    reviewGuidance = ReviewGuidance.MERGE_AFTER_REVIEW,
+    importance = Importance.HIGH,
+    executionPriority = CodemodExecutionPriority.HIGH)
+public final class SonarSQLInjectionIssueCodemod extends SonarRemediatingJavaParserChanger {
+
+  private final Remediator<Issue> remediationStrategy;
+  private final RuleIssue issues;
+
+  @Inject
+  public SonarSQLInjectionIssueCodemod(
+      @ProvidedSonarScan(ruleId = "javasecurity:S3649") final RuleIssue issues) {
+    super(GenericRemediationMetadata.SQL_INJECTION.reporter(), issues);
+    this.issues = Objects.requireNonNull(issues);
+    this.remediationStrategy =
+        new SearcherStrategyRemediator.Builder<Issue>()
+            .withSearcherStrategyPair(
+                new FixCandidateSearcher.Builder<Issue>()
+                    .withMatcher(
+                        n ->
+                            Optional.empty()
+                                // is the argument of the call
+                                .or(
+                                    () ->
+                                        Optional.of(n)
+                                            .map(
+                                                m ->
+                                                    m instanceof Expression ? (Expression) m : null)
+                                            .flatMap(ASTs::isArgumentOfMethodCall)
+                                            .filter(SQLInjectionFixComposer::match))
+                                .isPresent())
+                    .build(),
+                new SQLInjectionFixComposer())
+            .build();
+  }
+
+  @Override
+  public DetectorRule detectorRule() {
+    return new DetectorRule(
+        "javasecurity:S3649",
+        "Database queries should not be vulnerable to injection attacks",
+        "https://rules.sonarsource.com/java/RSPEC-3649/");
+  }
+
+  @Override
+  public CodemodFileScanningResult visit(
+      final CodemodInvocationContext context, final CompilationUnit cu) {
+    List<Issue> issuesForFile = issues.getResultsByPath(context.path());
+    return remediationStrategy.remediateAll(
+        cu,
+        context.path().toString(),
+        detectorRule(),
+        issuesForFile,
+        SonarFinding::getKey,
+        i -> i.getTextRange() != null ? i.getTextRange().getStartLine() : i.getLine(),
+        i -> Optional.ofNullable(i.getTextRange()).map(TextRange::getEndLine),
+        i -> Optional.ofNullable(i.getTextRange()).map(tr -> tr.getStartOffset() + 1));
+  }
+}

core-codemods/src/test/java/io/codemodder/codemods/sonar/SonarSQLInjectionCodemodTest.java → core-codemods/src/test/java/io/codemodder/codemods/sonar/SonarSQLInjectionCodemodsTest.java

@@ -4,11 +4,11 @@
 import io.codemodder.testutils.Metadata;
 import org.junit.jupiter.api.Nested;
 
-final class SonarSQLInjectionCodemodTest {
+final class SonarSQLInjectionCodemodsTest {
 
   @Nested
   @Metadata(
-      codemodType = SonarSQLInjectionCodemod.class,
+      codemodType = SonarSQLInjectionHotspotCodemod.class,
       testResourceDir = "sonar-sql-injection-s2077/unsupported",
       renameTestFile = "src/main/java/org/owasp/webgoat/container/users/UserService.java",
       expectingFailedFixesAtLines = {52}, // we don't support this method
@@ -17,17 +17,27 @@ class UnsupportedTest implements CodemodTestMixin {}
 
   @Nested
   @Metadata(
-      codemodType = SonarSQLInjectionCodemod.class,
+      codemodType = SonarSQLInjectionIssueCodemod.class,
+      testResourceDir = "sonar-sql-injection-s3649",
+      renameTestFile =
+          "src/main/java/org/owasp/webgoat/lessons/sqlinjection/advanced/SqlInjectionChallenge.java",
+      expectingFixesAtLines = {69},
+      dependencies = {})
+  class FromIssueRatherThanHotspotTest implements CodemodTestMixin {}
+
+  @Nested
+  @Metadata(
+      codemodType = SonarSQLInjectionHotspotCodemod.class,
       testResourceDir = "sonar-sql-injection-s2077/supported",
       renameTestFile =
           "src/main/java/org/owasp/webgoat/lessons/sqlinjection/advanced/SqlInjectionChallenge.java",
       expectingFixesAtLines = {69},
       dependencies = {})
-  class SupportedTest implements CodemodTestMixin {}
+  class SupportedHotspotTest implements CodemodTestMixin {}
 
   @Nested
   @Metadata(
-      codemodType = SonarSQLInjectionCodemod.class,
+      codemodType = SonarSQLInjectionHotspotCodemod.class,
       testResourceDir = "sonar-sql-injection-s2077/supportedTableInjection",
       renameTestFile = "core-codemods/src/main/java/io/codemodder/codemods/SQLTest.java",
       expectingFixesAtLines = {19, 25, 33, 40},
@@ -36,7 +46,7 @@ class SupportedTableInjectionTest implements CodemodTestMixin {}
 
   @Nested
   @Metadata(
-      codemodType = SonarSQLInjectionCodemod.class,
+      codemodType = SonarSQLInjectionHotspotCodemod.class,
       testResourceDir = "sonar-sql-injection-s2077/supportedMixedInjections",
       renameTestFile = "core-codemods/src/main/java/io/codemodder/codemods/SQLTestMixed.java",
       expectingFixesAtLines = {21},

core-codemods/src/test/resources/sonar-sql-injection-s3649/SqlInjectionChallenge.java.after

@@ -0,0 +1,106 @@
+/*
+ * This file is part of WebGoat, an Open Web Application Security Project utility. For details, please see http://www.owasp.org/
+ *
+ * Copyright (c) 2002 - 2019 Bruce Mayhew
+ *
+ * This program is free software; you can redistribute it and/or modify it under the terms of the
+ * GNU General Public License as published by the Free Software Foundation; either version 2 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without
+ * even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along with this program; if
+ * not, write to the Free Software Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA
+ * 02111-1307, USA.
+ *
+ * Getting Source ==============
+ *
+ * Source for this application is maintained at https://github.com/WebGoat/WebGoat, a repository for free software projects.
+ */
+
+package org.owasp.webgoat.lessons.sqlinjection.advanced;
+
+import java.sql.*;
+import java.sql.PreparedStatement;
+import lombok.extern.slf4j.Slf4j;
+import org.owasp.webgoat.container.LessonDataSource;
+import org.owasp.webgoat.container.assignments.AssignmentEndpoint;
+import org.owasp.webgoat.container.assignments.AssignmentHints;
+import org.owasp.webgoat.container.assignments.AttackResult;
+import org.springframework.util.StringUtils;
+import org.springframework.web.bind.annotation.PutMapping;
+import org.springframework.web.bind.annotation.RequestParam;
+import org.springframework.web.bind.annotation.ResponseBody;
+import org.springframework.web.bind.annotation.RestController;
+
+/**
+ * @author nbaars
+ * @since 4/8/17.
+ */
+@RestController
+@AssignmentHints(
+    value = {"SqlInjectionChallenge1", "SqlInjectionChallenge2", "SqlInjectionChallenge3"})
+@Slf4j
+public class SqlInjectionChallenge extends AssignmentEndpoint {
+
+  private final LessonDataSource dataSource;
+
+  public SqlInjectionChallenge(LessonDataSource dataSource) {
+    this.dataSource = dataSource;
+  }
+
+  @PutMapping("/SqlInjectionAdvanced/challenge")
+  // assignment path is bounded to class so we use different http method :-)
+  @ResponseBody
+  public AttackResult registerNewUser(
+      @RequestParam String username_reg,
+      @RequestParam String email_reg,
+      @RequestParam String password_reg)
+      throws Exception {
+    AttackResult attackResult = checkArguments(username_reg, email_reg, password_reg);
+
+    if (attackResult == null) {
+
+      try (Connection connection = dataSource.getConnection()) {
+        String checkUserQuery =
+            "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'")) {
+            attackResult = success(this).feedback("user.exists").build();
+          } else {
+            attackResult = failed(this).feedback("user.exists").feedbackArgs(username_reg).build();
+          }
+        } else {
+          PreparedStatement preparedStatement =
+              connection.prepareStatement("INSERT INTO sql_challenge_users VALUES (?, ?, ?)");
+          preparedStatement.setString(1, username_reg);
+          preparedStatement.setString(2, email_reg);
+          preparedStatement.setString(3, password_reg);
+          preparedStatement.execute();
+          attackResult = success(this).feedback("user.created").feedbackArgs(username_reg).build();
+        }
+      } catch (SQLException e) {
+        attackResult = failed(this).output("Something went wrong").build();
+      }
+    }
+    return attackResult;
+  }
+
+  private AttackResult checkArguments(String username_reg, String email_reg, String password_reg) {
+    if (StringUtils.isEmpty(username_reg)
+        || StringUtils.isEmpty(email_reg)
+        || StringUtils.isEmpty(password_reg)) {
+      return failed(this).feedback("input.invalid").build();
+    }
+    if (username_reg.length() > 250 || email_reg.length() > 30 || password_reg.length() > 30) {
+      return failed(this).feedback("input.invalid").build();
+    }
+    return null;
+  }
+}

core-codemods/src/test/resources/sonar-sql-injection-s3649/SqlInjectionChallenge.java.before

@@ -0,0 +1,104 @@
+/*
+ * This file is part of WebGoat, an Open Web Application Security Project utility. For details, please see http://www.owasp.org/
+ *
+ * Copyright (c) 2002 - 2019 Bruce Mayhew
+ *
+ * This program is free software; you can redistribute it and/or modify it under the terms of the
+ * GNU General Public License as published by the Free Software Foundation; either version 2 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without
+ * even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along with this program; if
+ * not, write to the Free Software Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA
+ * 02111-1307, USA.
+ *
+ * Getting Source ==============
+ *
+ * Source for this application is maintained at https://github.com/WebGoat/WebGoat, a repository for free software projects.
+ */
+
+package org.owasp.webgoat.lessons.sqlinjection.advanced;
+
+import java.sql.*;
+import lombok.extern.slf4j.Slf4j;
+import org.owasp.webgoat.container.LessonDataSource;
+import org.owasp.webgoat.container.assignments.AssignmentEndpoint;
+import org.owasp.webgoat.container.assignments.AssignmentHints;
+import org.owasp.webgoat.container.assignments.AttackResult;
+import org.springframework.util.StringUtils;
+import org.springframework.web.bind.annotation.PutMapping;
+import org.springframework.web.bind.annotation.RequestParam;
+import org.springframework.web.bind.annotation.ResponseBody;
+import org.springframework.web.bind.annotation.RestController;
+
+/**
+ * @author nbaars
+ * @since 4/8/17.
+ */
+@RestController
+@AssignmentHints(
+    value = {"SqlInjectionChallenge1", "SqlInjectionChallenge2", "SqlInjectionChallenge3"})
+@Slf4j
+public class SqlInjectionChallenge extends AssignmentEndpoint {
+
+  private final LessonDataSource dataSource;
+
+  public SqlInjectionChallenge(LessonDataSource dataSource) {
+    this.dataSource = dataSource;
+  }
+
+  @PutMapping("/SqlInjectionAdvanced/challenge")
+  // assignment path is bounded to class so we use different http method :-)
+  @ResponseBody
+  public AttackResult registerNewUser(
+      @RequestParam String username_reg,
+      @RequestParam String email_reg,
+      @RequestParam String password_reg)
+      throws Exception {
+    AttackResult attackResult = checkArguments(username_reg, email_reg, password_reg);
+
+    if (attackResult == null) {
+
+      try (Connection connection = dataSource.getConnection()) {
+        String checkUserQuery =
+            "select userid from sql_challenge_users where userid = '" + username_reg + "'";
+        Statement statement = connection.createStatement();
+        ResultSet resultSet = statement.executeQuery(checkUserQuery);
+
+        if (resultSet.next()) {
+          if (username_reg.contains("tom'")) {
+            attackResult = success(this).feedback("user.exists").build();
+          } else {
+            attackResult = failed(this).feedback("user.exists").feedbackArgs(username_reg).build();
+          }
+        } else {
+          PreparedStatement preparedStatement =
+              connection.prepareStatement("INSERT INTO sql_challenge_users VALUES (?, ?, ?)");
+          preparedStatement.setString(1, username_reg);
+          preparedStatement.setString(2, email_reg);
+          preparedStatement.setString(3, password_reg);
+          preparedStatement.execute();
+          attackResult = success(this).feedback("user.created").feedbackArgs(username_reg).build();
+        }
+      } catch (SQLException e) {
+        attackResult = failed(this).output("Something went wrong").build();
+      }
+    }
+    return attackResult;
+  }
+
+  private AttackResult checkArguments(String username_reg, String email_reg, String password_reg) {
+    if (StringUtils.isEmpty(username_reg)
+        || StringUtils.isEmpty(email_reg)
+        || StringUtils.isEmpty(password_reg)) {
+      return failed(this).feedback("input.invalid").build();
+    }
+    if (username_reg.length() > 250 || email_reg.length() > 30 || password_reg.length() > 30) {
+      return failed(this).feedback("input.invalid").build();
+    }
+    return null;
+  }
+}