From: Robert Varga <robert.varga@pantheon.tech>
Date: Wed, 16 Nov 2022 17:27:12 +0000 (+0100)
Subject: Use prepareStatement() in UserStore.deleteUser()
X-Git-Tag: v0.15.8~4
X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?a=commitdiff_plain;h=5fd03f1f7074bbff907876e0853f8202076e7152;p=aaa.git

Use prepareStatement() in UserStore.deleteUser()

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)
---

diff --git a/aaa-idm-store-h2/src/main/java/org/opendaylight/aaa/datastore/h2/UserStore.java b/aaa-idm-store-h2/src/main/java/org/opendaylight/aaa/datastore/h2/UserStore.java
index e434d17a6..943445d49 100644
--- a/aaa-idm-store-h2/src/main/java/org/opendaylight/aaa/datastore/h2/UserStore.java
+++ b/aaa-idm-store-h2/src/main/java/org/opendaylight/aaa/datastore/h2/UserStore.java
@@ -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) {