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.17.0~8
X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?a=commitdiff_plain;h=9b912d4d433469b83f097fa76e203d7b97f44552;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>
---

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 7dc4acd4b..d97703632 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
@@ -10,11 +10,9 @@ package org.opendaylight.aaa.datastore.h2;
 import static java.util.Objects.requireNonNull;
 
 import com.google.common.annotations.VisibleForTesting;
-import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.sql.ResultSet;
 import java.sql.SQLException;
 import java.sql.Statement;
-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;
@@ -220,22 +218,19 @@ final class UserStore extends AbstractStore<User> {
         return savedUser;
     }
 
-    @SuppressFBWarnings(value = "SQL_NONCONSTANT_STRING_PASSED_TO_EXECUTE", justification = "Weird original code")
     User deleteUser(final String userid) throws StoreException {
-        // FIXME: remove this once we have a more modern H2
-        final var escaped = StringEscapeUtils.escapeHtml4(userid);
-        final var savedUser = getUser(escaped);
+        final var savedUser = getUser(userid);
         if (savedUser == null) {
             return null;
         }
 
         try (var conn = dbConnect();
-             var stmt = conn.createStatement()) {
+             var stmt = conn.prepareStatement("DELETE FROM " + TABLE + " WHERE " + COL_ID + " = ?")) {
             // FIXME: prepare statement instead
-            final var query = String.format("DELETE FROM " + TABLE + " WHERE " + COL_ID + " = '%s'", escaped);
-            LOG.debug("deleteUser() request: {}", query);
+            stmt.setString(1, userid);
+            LOG.debug("deleteUser() request: {}", stmt);
 
-            int deleteCount = stmt.executeUpdate(query);
+            int deleteCount = stmt.executeUpdate();
             LOG.debug("deleted {} records", deleteCount);
             return savedUser;
         } catch (SQLException s) {