From 22a9aef7cfc9ef7a0a2f592f2f39b818727dd994 Mon Sep 17 00:00:00 2001
From: Remi Gacogne <remi.gacogne@powerdns.com>
Date: Thu, 21 Mar 2024 09:22:24 +0100
Subject: [PATCH] auth: Wrap SSql pointers in a unique pointer earlier

Using a unique pointer from the beginning makes the ownership of the
pointer more clear, and prevents leaking the database connection and
associated memory if an exception is raised right away.
---
 modules/gmysqlbackend/gmysqlbackend.cc     | 22 +++++++++++-----------
 modules/godbcbackend/godbcbackend.cc       |  2 +-
 modules/gpgsqlbackend/gpgsqlbackend.cc     | 14 +++++++-------
 modules/gsqlite3backend/gsqlite3backend.cc |  6 +++---
 pdns/backends/gsql/gsqlbackend.hh          |  4 ++--
 5 files changed, 24 insertions(+), 24 deletions(-)

diff --git a/modules/gmysqlbackend/gmysqlbackend.cc b/modules/gmysqlbackend/gmysqlbackend.cc
index 869f819db2..7aac1044ae 100644
--- a/modules/gmysqlbackend/gmysqlbackend.cc
+++ b/modules/gmysqlbackend/gmysqlbackend.cc
@@ -52,17 +52,17 @@ gMySQLBackend::gMySQLBackend(const string& mode, const string& suffix) :
 
 void gMySQLBackend::reconnect()
 {
-  setDB(new SMySQL(getArg("dbname"),
-                   getArg("host"),
-                   getArgAsNum("port"),
-                   getArg("socket"),
-                   getArg("user"),
-                   getArg("password"),
-                   getArg("group"),
-                   mustDo("innodb-read-committed"),
-                   getArgAsNum("timeout"),
-                   mustDo("thread-cleanup"),
-                   mustDo("ssl")));
+  setDB(std::unique_ptr<SSql>(new SMySQL(getArg("dbname"),
+                                         getArg("host"),
+                                         getArgAsNum("port"),
+                                         getArg("socket"),
+                                         getArg("user"),
+                                         getArg("password"),
+                                         getArg("group"),
+                                         mustDo("innodb-read-committed"),
+                                         getArgAsNum("timeout"),
+                                         mustDo("thread-cleanup"),
+                                         mustDo("ssl"))));
   allocateStatements();
 }
 
diff --git a/modules/godbcbackend/godbcbackend.cc b/modules/godbcbackend/godbcbackend.cc
index 83123d8b03..39b34510b1 100644
--- a/modules/godbcbackend/godbcbackend.cc
+++ b/modules/godbcbackend/godbcbackend.cc
@@ -38,7 +38,7 @@ gODBCBackend::gODBCBackend(const std::string& mode, const std::string& suffix) :
   GSQLBackend(mode, suffix)
 {
   try {
-    setDB(new SODBC(getArg("datasource"), getArg("username"), getArg("password")));
+    setDB(std::unique_ptr<SSql>(new SODBC(getArg("datasource"), getArg("username"), getArg("password"))));
   }
   catch (SSqlException& e) {
     g_log << Logger::Error << mode << " Connection failed: " << e.txtReason() << std::endl;
diff --git a/modules/gpgsqlbackend/gpgsqlbackend.cc b/modules/gpgsqlbackend/gpgsqlbackend.cc
index f69828aeaf..481ac8fd21 100644
--- a/modules/gpgsqlbackend/gpgsqlbackend.cc
+++ b/modules/gpgsqlbackend/gpgsqlbackend.cc
@@ -40,13 +40,13 @@ gPgSQLBackend::gPgSQLBackend(const string& mode, const string& suffix) :
   GSQLBackend(mode, suffix)
 {
   try {
-    setDB(new SPgSQL(getArg("dbname"),
-                     getArg("host"),
-                     getArg("port"),
-                     getArg("user"),
-                     getArg("password"),
-                     getArg("extra-connection-parameters"),
-                     mustDo("prepared-statements")));
+    setDB(std::unique_ptr<SSql>(new SPgSQL(getArg("dbname"),
+                                           getArg("host"),
+                                           getArg("port"),
+                                           getArg("user"),
+                                           getArg("password"),
+                                           getArg("extra-connection-parameters"),
+                                           mustDo("prepared-statements"))));
   }
 
   catch (SSqlException& e) {
diff --git a/modules/gsqlite3backend/gsqlite3backend.cc b/modules/gsqlite3backend/gsqlite3backend.cc
index 829213d6c8..f54491c8d4 100644
--- a/modules/gsqlite3backend/gsqlite3backend.cc
+++ b/modules/gsqlite3backend/gsqlite3backend.cc
@@ -43,15 +43,15 @@ gSQLite3Backend::gSQLite3Backend(const std::string& mode, const std::string& suf
   GSQLBackend(mode, suffix)
 {
   try {
-    SSQLite3* ptr = new SSQLite3(getArg("database"), getArg("pragma-journal-mode"));
-    setDB(ptr);
-    allocateStatements();
+    auto ptr = std::unique_ptr<SSql>(new SSQLite3(getArg("database"), getArg("pragma-journal-mode")));
     if (!getArg("pragma-synchronous").empty()) {
       ptr->execute("PRAGMA synchronous=" + getArg("pragma-synchronous"));
     }
     if (mustDo("pragma-foreign-keys")) {
       ptr->execute("PRAGMA foreign_keys = 1");
     }
+    setDB(std::move(ptr));
+    allocateStatements();
   }
   catch (SSqlException& e) {
     g_log << Logger::Error << mode << ": connection failed: " << e.txtReason() << std::endl;
diff --git a/pdns/backends/gsql/gsqlbackend.hh b/pdns/backends/gsql/gsqlbackend.hh
index 8f92d5fb8c..e54e5b2a3b 100644
--- a/pdns/backends/gsql/gsqlbackend.hh
+++ b/pdns/backends/gsql/gsqlbackend.hh
@@ -42,10 +42,10 @@ public:
     d_db.reset();
   }
 
-  void setDB(SSql *db)
+  void setDB(std::unique_ptr<SSql>&& database)
   {
     freeStatements();
-    d_db=std::unique_ptr<SSql>(db);
+    d_db = std::move(database);
     if (d_db) {
       d_db->setLog(::arg().mustDo("query-logging"));
     }
-- 
2.47.2