Fixed unregistering on the server-side.

When registering or logging, The client is hashing the password
for sending it safely. And the server is hashing it also
to store it the same way. Hence, the password ends hashed twice,
which is correct because the server can't trust the client anyway.

At unregister attempt, the server wasn't hashing the password
before comparing it.

Also while on it, I made the corresponding SQL query use the try catch
method and only delete the account in memory when it's also done
on the Db.

Reviewed-by: thorbjorn, Freeyorp.

2 files changed, 20 insertions, 9 deletions

diff --git a/src/account-server/accounthandler.cpp b/src/account-server/accounthandler.cpp
index 8fa0576c..d76a6b1b 100644
--- a/src/account-server/accounthandler.cpp
+++ b/src/account-server/accounthandler.cpp
@@ -535,8 +535,6 @@ void AccountHandler::handleUnregisterMessage(AccountClient &client,
                                              MessageIn &msg)
 {
     LOG_DEBUG("AccountHandler::handleUnregisterMessage");
-    std::string username = msg.readString();
-    std::string password = msg.readString();
 
     MessageOut reply(APMSG_UNREGISTER_RESPONSE);
 
@@ -547,6 +545,9 @@ void AccountHandler::handleUnregisterMessage(AccountClient &client,
         return;
     }
 
+    std::string username = msg.readString();
+    std::string password = msg.readString();
+
     if (stringFilter->findDoubleQuotes(username))
     {
         reply.writeInt8(ERRMSG_INVALID_ARGUMENT);
@@ -557,7 +558,7 @@ void AccountHandler::handleUnregisterMessage(AccountClient &client,
     // See whether the account exists
     Account *acc = storage->getAccount(username);
 
-    if (!acc || acc->getPassword() != password)
+    if (!acc || acc->getPassword() != sha256(password))
     {
         reply.writeInt8(ERRMSG_INVALID_ARGUMENT);
         client.send(reply);
diff --git a/src/account-server/storage.cpp b/src/account-server/storage.cpp
index 84dfedc4..20f9fe5a 100644
--- a/src/account-server/storage.cpp
+++ b/src/account-server/storage.cpp
@@ -1037,14 +1037,24 @@ void Storage::flush(Account *account)
  */
 void Storage::delAccount(Account *account)
 {
-    account->setCharacters(Characters());
+    // Sync the account info into the database.
     flush(account);
 
-    // delete the account.
-    std::ostringstream sql;
-    sql << "delete from " << ACCOUNTS_TBL_NAME
-        << " where id = '" << account->getID() << "';";
-    mDb->execSql(sql.str());
+    try
+    {
+        // Delete the account.
+        std::ostringstream sql;
+        sql << "delete from " << ACCOUNTS_TBL_NAME
+            << " where id = '" << account->getID() << "';";
+        mDb->execSql(sql.str());
+
+        // Remove the account's characters.
+        account->setCharacters(Characters());
+    }
+    catch (const std::exception &e)
+    {
+        LOG_ERROR("ERROR in DALStorage::delAccount: " << e.what());
+    }
 }
 
 /**