From 9b912d4d433469b83f097fa76e203d7b97f44552 Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Wed, 16 Nov 2022 18:27:12 +0100 Subject: [PATCH] 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 --- .../opendaylight/aaa/datastore/h2/UserStore.java | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) 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 { 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) { -- 2.36.6