Core/Commands: Remove all active bans before adding new one, prevents multiple active bans for one character/account

Core/DBLayer: Converted most of ban related queries into prepared statement (might have missed some)

Closes issue #4218.

--HG--
branch : trunk

5 files changed, 63 insertions, 26 deletions

diff --git a/src/server/game/World/World.cpp b/src/server/game/World/World.cpp
index 065418bfd6b..ea50e28ed36 100644
--- a/src/server/game/World/World.cpp
+++ b/src/server/game/World/World.cpp
@@ -2202,29 +2202,36 @@ void World::KickAllLess(AccountTypes sec)
 /// Ban an account or ban an IP address, duration will be parsed using TimeStringToSecs if it is positive, otherwise permban
 BanReturn World::BanAccount(BanMode mode, std::string nameOrIP, std::string duration, std::string reason, std::string author)
 {
-    LoginDatabase.escape_string(nameOrIP);
-    LoginDatabase.escape_string(reason);
-    std::string safe_author = author;
-    LoginDatabase.escape_string(safe_author);
-
     uint32 duration_secs = TimeStringToSecs(duration);
-    QueryResult resultAccounts = QueryResult(NULL);                     //used for kicking
+    PreparedQueryResult resultAccounts = PreparedQueryResult(NULL); //used for kicking
+    PreparedStatement* stmt = NULL;
 
     ///- Update the database with ban information
     switch(mode)
     {
         case BAN_IP:
-            //No SQL injection as strings are escaped
-            resultAccounts = LoginDatabase.PQuery("SELECT id FROM account WHERE last_ip = '%s'", nameOrIP.c_str());
-            LoginDatabase.PExecute("INSERT INTO ip_banned VALUES ('%s',UNIX_TIMESTAMP(),UNIX_TIMESTAMP()+%u,'%s','%s')", nameOrIP.c_str(), duration_secs, safe_author.c_str(), reason.c_str());
+            // No SQL injection with prepared statements
+            stmt = LoginDatabase.GetPreparedStatement(LOGIN_GET_ACCOUNT_BY_IP);
+            stmt->setString(0, nameOrIP);
+            resultAccounts = LoginDatabase.Query(stmt);
+            stmt = LoginDatabase.GetPreparedStatement(LOGIN_SET_IP_BANNED);
+            stmt->setString(0, nameOrIP);
+            stmt->setUInt32(1, duration_secs);
+            stmt->setString(2, author);
+            stmt->setString(3, reason);
+            LoginDatabase.Execute(stmt);
             break;
         case BAN_ACCOUNT:
-            //No SQL injection as string is escaped
-            resultAccounts = LoginDatabase.PQuery("SELECT id FROM account WHERE username = '%s'", nameOrIP.c_str());
+            // No SQL injection with prepared statements
+            stmt = LoginDatabase.GetPreparedStatement(LOGIN_GET_ACCIDBYNAME);
+            stmt->setString(0, nameOrIP);
+            resultAccounts = LoginDatabase.Query(stmt);
             break;
         case BAN_CHARACTER:
-            //No SQL injection as string is escaped
-            resultAccounts = CharacterDatabase.PQuery("SELECT account FROM characters WHERE name = '%s'", nameOrIP.c_str());
+            // No SQL injection with prepared statements
+            stmt = CharacterDatabase.GetPreparedStatement(CHAR_GET_ACCOUNT_BY_NAME);
+            stmt->setString(0, nameOrIP);
+            resultAccounts = CharacterDatabase.Query(stmt);
             break;
         default:
             return BAN_SYNTAX_ERROR;
@@ -2235,10 +2242,11 @@ BanReturn World::BanAccount(BanMode mode, std::string nameOrIP, std::string dura
         if (mode == BAN_IP)
             return BAN_SUCCESS;                             // ip correctly banned but nobody affected (yet)
         else
-            return BAN_NOTFOUND;                                // Nobody to ban
+            return BAN_NOTFOUND;                            // Nobody to ban
     }
 
     ///- Disconnect all affected players (for IP it can be several)
+    SQLTransaction trans = LoginDatabase.BeginTransaction();
     do
     {
         Field* fieldsAccount = resultAccounts->Fetch();
@@ -2246,9 +2254,17 @@ BanReturn World::BanAccount(BanMode mode, std::string nameOrIP, std::string dura
 
         if (mode != BAN_IP)
         {
-            //No SQL injection as strings are escaped
-            LoginDatabase.PExecute("INSERT INTO account_banned VALUES ('%u', UNIX_TIMESTAMP(), UNIX_TIMESTAMP()+%u, '%s', '%s', '1')",
-                account,duration_secs,safe_author.c_str(),reason.c_str());
+            // make sure there is only one active ban
+            stmt = LoginDatabase.GetPreparedStatement(LOGIN_SET_ACCOUNT_NOT_BANNED);
+            stmt->setUInt32(0, account);
+            trans->Append(stmt);
+            // No SQL injection with prepared statements
+            stmt = LoginDatabase.GetPreparedStatement(LOGIN_SET_ACCOUNT_BANNED);
+            stmt->setUInt32(0, account);
+            stmt->setUInt32(1, duration_secs);
+            stmt->setString(2, author);
+            stmt->setString(3, reason);
+            trans->Append(stmt);
         }
 
         if (WorldSession* sess = FindSession(account))
@@ -2256,30 +2272,36 @@ BanReturn World::BanAccount(BanMode mode, std::string nameOrIP, std::string dura
                 sess->KickPlayer();
     } while (resultAccounts->NextRow());
 
+    LoginDatabase.CommitTransaction(trans);
+
     return BAN_SUCCESS;
 }
 
 /// Remove a ban from an account or IP address
 bool World::RemoveBanAccount(BanMode mode, std::string nameOrIP)
 {
+    PreparedStatement* stmt = NULL;
     if (mode == BAN_IP)
     {
-        LoginDatabase.escape_string(nameOrIP);
-        LoginDatabase.PExecute("DELETE FROM ip_banned WHERE ip = '%s'",nameOrIP.c_str());
+        stmt = LoginDatabase.GetPreparedStatement(LOGIN_SET_IP_NOT_BANNED);
+        stmt->setString(0, nameOrIP);
+        LoginDatabase.Execute(stmt);
     }
     else
     {
         uint32 account = 0;
         if (mode == BAN_ACCOUNT)
-            account = sAccountMgr.GetId (nameOrIP);
+            account = sAccountMgr.GetId(nameOrIP);
         else if (mode == BAN_CHARACTER)
-            account = sObjectMgr.GetPlayerAccountIdByPlayerName (nameOrIP);
+            account = sObjectMgr.GetPlayerAccountIdByPlayerName(nameOrIP);
 
         if (!account)
             return false;
 
         //NO SQL injection as account is uint32
-        LoginDatabase.PExecute("UPDATE account_banned SET active = '0' WHERE id = '%u'",account);
+        stmt = LoginDatabase.GetPreparedStatement(LOGIN_SET_ACCOUNT_NOT_BANNED);
+        stmt->setUInt32(0, account);
+        LoginDatabase.Execute(stmt);
     }
     return true;
 }
@@ -2307,7 +2329,12 @@ BanReturn World::BanCharacter(std::string name, std::string duration, std::strin
     else
         guid = pBanned->GetGUIDLow();
 
-    PreparedStatement* stmt = CharacterDatabase.GetPreparedStatement(CHAR_ADD_BAN);
+    // make sure there is only one active ban
+    PreparedStatement* stmt = CharacterDatabase.GetPreparedStatement(CHAR_SET_NOT_BANNED);
+    stmt->setUInt32(0, guid);
+    CharacterDatabase.Execute(stmt);
+
+    stmt = CharacterDatabase.GetPreparedStatement(CHAR_ADD_BAN);
     stmt->setUInt32(0, guid);
     stmt->setUInt32(1, duration_secs);
     stmt->setString(2, author);
diff --git a/src/server/shared/Database/Implementation/CharacterDatabase.cpp b/src/server/shared/Database/Implementation/CharacterDatabase.cpp
index ee02f8ca2d8..3e6f1f5c74a 100644
--- a/src/server/shared/Database/Implementation/CharacterDatabase.cpp
+++ b/src/server/shared/Database/Implementation/CharacterDatabase.cpp
@@ -42,7 +42,7 @@ bool CharacterDatabaseConnection::Open(const std::string& infoString)
     PrepareStatement(CHAR_DEL_EXPIRED_BANS, "UPDATE character_banned SET active = 0 WHERE unbandate <= UNIX_TIMESTAMP() AND unbandate <> bandate");
     PrepareStatement(CHAR_GET_GUID_BY_NAME, "SELECT guid FROM characters WHERE name = ?");
     PrepareStatement(CHAR_ADD_BAN, "INSERT INTO character_banned VALUES (?, UNIX_TIMESTAMP(), UNIX_TIMESTAMP()+?, ?, ?, 1)");
-    PrepareStatement(CHAR_SET_NOT_BANNED, "UPDATE character_banned SET active = 0 WHERE guid = ?");
+    PrepareStatement(CHAR_SET_NOT_BANNED, "UPDATE character_banned SET active = 0 WHERE guid = ? AND active != 0");
     PrepareStatement(CHAR_GET_BANINFO, "SELECT FROM_UNIXTIME(bandate), unbandate-bandate, active, unbandate, banreason, bannedby FROM character_banned WHERE guid = ? ORDER BY bandate ASC");
     PrepareStatement(CHAR_GET_GUID_BY_NAME_FILTER, "SELECT guid, name FROM characters WHERE name LIKE CONCAT('%', ?, '%')");
     PrepareStatement(CHAR_GET_BANINFO_LIST, "SELECT bandate, unbandate, bannedby, banreason FROM character_banned WHERE guid = ? ORDER BY unbandate");
@@ -96,6 +96,7 @@ bool CharacterDatabaseConnection::Open(const std::string& infoString)
     PrepareStatement(CHAR_ADD_ITEM_BOP_TRADE, "INSERT INTO item_soulbound_trade_data VALUES (?, ?)");
     PrepareStatement(CHAR_DEL_INVENTORY_ITEM, "DELETE FROM character_inventory WHERE item = ?");
     PrepareStatement(CHAR_DEL_ITEM_INSTANCE, "DELETE FROM item_instance WHERE guid = ?");
+    PrepareStatement(CHAR_GET_ACCOUNT_BY_NAME, "SELECT account FROM characters WHERE name = ?");
 
     return true;
 }
diff --git a/src/server/shared/Database/Implementation/CharacterDatabase.h b/src/server/shared/Database/Implementation/CharacterDatabase.h
index 4628bfafb83..27aa20b9a13 100644
--- a/src/server/shared/Database/Implementation/CharacterDatabase.h
+++ b/src/server/shared/Database/Implementation/CharacterDatabase.h
@@ -102,6 +102,7 @@ enum CharacterDatabaseStatements
     CHAR_ADD_ITEM_BOP_TRADE,
     CHAR_DEL_INVENTORY_ITEM,
     CHAR_DEL_ITEM_INSTANCE,
+    CHAR_GET_ACCOUNT_BY_NAME,
     MAX_CHARACTERDATABASE_STATEMENTS,
 };
 
diff --git a/src/server/shared/Database/Implementation/LoginDatabase.cpp b/src/server/shared/Database/Implementation/LoginDatabase.cpp
index 5151f72297d..5a662af7882 100644
--- a/src/server/shared/Database/Implementation/LoginDatabase.cpp
+++ b/src/server/shared/Database/Implementation/LoginDatabase.cpp
@@ -43,9 +43,13 @@ bool LoginDatabaseConnection::Open(const std::string& infoString)
     PrepareStatement(LOGIN_GET_LOGONCHALLENGE, "SELECT a.sha_pass_hash,a.id,a.locked,a.last_ip,aa.gmlevel,a.v,a.s FROM account a LEFT JOIN account_access aa ON (a.id = aa.id) WHERE a.username = ?");
     PrepareStatement(LOGIN_SET_FAILEDLOGINS, "UPDATE account SET failed_logins = failed_logins + 1 WHERE username = ?");
     PrepareStatement(LOGIN_GET_FAILEDLOGINS, "SELECT id, failed_logins FROM account WHERE username = ?");
-
     PrepareStatement(LOGIN_GET_ACCIDBYNAME, "SELECT id FROM account WHERE username = ?");
     PrepareStatement(LOGIN_GET_NUMCHARSONREALM, "SELECT numchars FROM realmcharacters WHERE realmid = ? AND acctid= ?");
+    PrepareStatement(LOGIN_GET_ACCOUNT_BY_IP, "SELECT id FROM account WHERE last_ip = ?");
+    PrepareStatement(LOGIN_SET_IP_BANNED, "INSERT INTO ip_banned VALUES (?, UNIX_TIMESTAMP(), UNIX_TIMESTAMP()+?, ?, ?)");
+    PrepareStatement(LOGIN_SET_IP_NOT_BANNED, "DELETE FROM ip_banned WHERE ip = ?");
+    PrepareStatement(LOGIN_SET_ACCOUNT_BANNED, "INSERT INTO account_banned VALUES (?, UNIX_TIMESTAMP(), UNIX_TIMESTAMP()+?, ?, ?, 1)");
+    PrepareStatement(LOGIN_SET_ACCOUNT_NOT_BANNED, "UPDATE account_banned SET active = 0 WHERE id = ? AND active != 0");
 
     return true;
 }
diff --git a/src/server/shared/Database/Implementation/LoginDatabase.h b/src/server/shared/Database/Implementation/LoginDatabase.h
index 4792858d50e..69674b2e354 100644
--- a/src/server/shared/Database/Implementation/LoginDatabase.h
+++ b/src/server/shared/Database/Implementation/LoginDatabase.h
@@ -56,9 +56,13 @@ enum LoginDatabaseStatements
     LOGIN_GET_LOGONCHALLENGE,
     LOGIN_SET_FAILEDLOGINS,
     LOGIN_GET_FAILEDLOGINS,
-
     LOGIN_GET_ACCIDBYNAME,
     LOGIN_GET_NUMCHARSONREALM,
+    LOGIN_GET_ACCOUNT_BY_IP,
+    LOGIN_SET_IP_BANNED,
+    LOGIN_SET_IP_NOT_BANNED,
+    LOGIN_SET_ACCOUNT_BANNED,
+    LOGIN_SET_ACCOUNT_NOT_BANNED,
 
     MAX_LOGINDATABASE_STATEMENTS,
 };