From 9c5c61991ac0dd1fb7d3f6b7bfed36eefe6f870d 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 (cherry picked from commit 9b912d4d433469b83f097fa76e203d7b97f44552) --- .../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