Use prepareStatement() in DomainStore.deleteDomain() 71/103271/1
authorRobert Varga <robert.varga@pantheon.tech>
Wed, 16 Nov 2022 17:24:46 +0000 (18:24 +0100)
committerRobert Varga <robert.varga@pantheon.tech>
Wed, 16 Nov 2022 17:53:17 +0000 (18:53 +0100)
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)

aaa-idm-store-h2/src/main/java/org/opendaylight/aaa/datastore/h2/DomainStore.java

index d6e7debc49041691e2d5978f645701ae321d1334..9b7aecce9e335ddc9422a9245f1be11f1791fed0 100644 (file)
@@ -9,13 +9,10 @@ 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 org.apache.commons.text.StringEscapeUtils;
 import org.opendaylight.aaa.api.model.Domain;
 import org.opendaylight.aaa.api.model.Domains;
 import org.slf4j.Logger;
@@ -116,7 +113,7 @@ public class DomainStore extends AbstractStore<Domain> {
     }
 
     protected Domain putDomain(final Domain domain) throws StoreException {
-        Domain savedDomain = this.getDomain(domain.getDomainid());
+        Domain savedDomain = getDomain(domain.getDomainid());
         if (savedDomain == null) {
             return null;
         }
@@ -147,17 +144,16 @@ public class DomainStore extends AbstractStore<Domain> {
         return savedDomain;
     }
 
-    @SuppressFBWarnings("SQL_NONCONSTANT_STRING_PASSED_TO_EXECUTE")
-    protected Domain deleteDomain(String domainid) throws StoreException {
-        domainid = StringEscapeUtils.escapeHtml4(domainid);
-        Domain deletedDomain = this.getDomain(domainid);
+    protected Domain deleteDomain(final String domainid) throws StoreException {
+        Domain deletedDomain = getDomain(domainid);
         if (deletedDomain == null) {
             return null;
         }
-        String query = String.format("DELETE FROM DOMAINS WHERE domainid = '%s'", domainid);
+        String query = "DELETE FROM DOMAINS WHERE domainid = ?";
         try (Connection conn = dbConnect();
-             Statement statement = conn.createStatement()) {
-            int deleteCount = statement.executeUpdate(query);
+             PreparedStatement statement = conn.prepareStatement(query)) {
+            statement.setString(1, domainid);
+            int deleteCount = statement.executeUpdate();
             LOG.debug("deleted {} records", deleteCount);
             return deletedDomain;
         } catch (SQLException e) {