Use prepareStatement() in UserStore.deleteUser() 72/103272/1
authorRobert Varga <robert.varga@pantheon.tech>
Wed, 16 Nov 2022 17:27:12 +0000 (18:27 +0100)
committerRobert Varga <robert.varga@pantheon.tech>
Wed, 16 Nov 2022 17:53:17 +0000 (18:53 +0100)
The conversion to prepared statements has not dealt with the delete
function, leaving the ability to wipe the entire UserStore with SQL
injection. Fix this by using a proper prepared statement.

JIRA: AAA-241
Change-Id: Ie3d9a8eae815fab457809f3d2cd3577d38bd0207
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
(cherry picked from commit 9b912d4d433469b83f097fa76e203d7b97f44552)

aaa-idm-store-h2/src/main/java/org/opendaylight/aaa/datastore/h2/UserStore.java

index e434d17a66540ee493d7914b690559dcf2a3a7e7..943445d493ab2eeccf6dcfe4bbe110ac7aa7fc39 100644 (file)
@@ -9,14 +9,11 @@ package org.opendaylight.aaa.datastore.h2;
 
 import static java.util.Objects.requireNonNull;
 
-import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.sql.Connection;
 import java.sql.PreparedStatement;
 import java.sql.ResultSet;
 import java.sql.SQLException;
-import java.sql.Statement;
 import java.util.Objects;
-import org.apache.commons.text.StringEscapeUtils;
 import org.opendaylight.aaa.api.IDMStoreUtil;
 import org.opendaylight.aaa.api.model.User;
 import org.opendaylight.aaa.api.model.Users;
@@ -143,7 +140,7 @@ public class UserStore extends AbstractStore<User> {
 
     public User putUser(final User user) throws StoreException {
 
-        User savedUser = this.getUser(user.getUserid());
+        User savedUser = getUser(user.getUserid());
         if (savedUser == null) {
             return null;
         }
@@ -186,17 +183,16 @@ public class UserStore extends AbstractStore<User> {
         return savedUser;
     }
 
-    @SuppressFBWarnings("SQL_NONCONSTANT_STRING_PASSED_TO_EXECUTE")
-    protected User deleteUser(String userid) throws StoreException {
-        userid = StringEscapeUtils.escapeHtml4(userid);
-        User savedUser = this.getUser(userid);
+    protected User deleteUser(final String userid) throws StoreException {
+        User savedUser = getUser(userid);
         if (savedUser == null) {
             return null;
         }
 
-        String query = String.format("DELETE FROM USERS WHERE userid = '%s'", userid);
-        try (Connection conn = dbConnect(); Statement statement = conn.createStatement()) {
-            int deleteCount = statement.executeUpdate(query);
+        String query = "DELETE FROM USERS WHERE userid = ?";
+        try (Connection conn = dbConnect(); PreparedStatement statement = conn.prepareStatement(query)) {
+            statement.setString(1, userid);
+            int deleteCount = statement.executeUpdate();
             LOG.debug("deleted {} records", deleteCount);
             return savedUser;
         } catch (SQLException s) {