From: Robert Varga <robert.varga@pantheon.tech>
Date: Wed, 16 Nov 2022 17:24:46 +0000 (+0100)
Subject: Use prepareStatement() in DomainStore.deleteDomain()
X-Git-Tag: v0.16.5~6
X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?a=commitdiff_plain;h=aa3596eae57d4a0c993305077938255055c9efca;p=aaa.git

Use prepareStatement() in DomainStore.deleteDomain()

The conversion to prepared statements has not dealt with the delete
function, leaving the ability to wipe the entire DomainStore with SQL
injection. Fix this by using a proper prepared statement.

JIRA: AAA-240
Change-Id: I4650e4561482864c90df737e964dcc5514221a15
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
(cherry picked from commit 11295189db80dd45fb0c460d9e9cb3598ed7f229)
---

diff --git a/aaa-idm-store-h2/src/main/java/org/opendaylight/aaa/datastore/h2/DomainStore.java b/aaa-idm-store-h2/src/main/java/org/opendaylight/aaa/datastore/h2/DomainStore.java
index cbed2d477..a2df376f7 100644
--- a/aaa-idm-store-h2/src/main/java/org/opendaylight/aaa/datastore/h2/DomainStore.java
+++ b/aaa-idm-store-h2/src/main/java/org/opendaylight/aaa/datastore/h2/DomainStore.java
@@ -11,11 +11,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.eclipse.jdt.annotation.NonNull;
 import org.opendaylight.aaa.api.model.Domain;
 import org.opendaylight.aaa.api.model.Domains;
@@ -182,22 +180,18 @@ final class DomainStore extends AbstractStore<Domain> {
         return savedDomain;
     }
 
-    @SuppressFBWarnings(value = "SQL_NONCONSTANT_STRING_PASSED_TO_EXECUTE", justification = "Weird original code")
     Domain deleteDomain(final String domainid) throws StoreException {
-        // FIXME: remove this once we have a more modern H2
-        final String escaped = StringEscapeUtils.escapeHtml4(domainid);
-        final var deletedDomain = getDomain(escaped);
+        final var deletedDomain = getDomain(domainid);
         if (deletedDomain == null) {
             return null;
         }
 
         try (var conn = dbConnect();
-             var stmt = conn.createStatement()) {
-            // FIXME: prepare statement instead
-            final String query = String.format("DELETE FROM " + TABLE + " WHERE " + COL_ID + " = '%s'", escaped);
-            LOG.debug("deleteDomain() request: {}", query);
+             var stmt = conn.prepareStatement("DELETE FROM " + TABLE + " WHERE " + COL_ID + " = ?")) {
+            stmt.setString(1, domainid);
 
-            int deleteCount = stmt.executeUpdate(query);
+            LOG.debug("deleteDomain() request: {}", stmt);
+            int deleteCount = stmt.executeUpdate();
             LOG.debug("deleted {} records", deleteCount);
             return deletedDomain;
         } catch (SQLException e) {