/cpw/tomkiewicz/masterpassword: 21aa923e62ad: Fix use-after-free

[Tomasz Wasilczyk]

Changeset: 21aa923e62ad0aa32f46638ea9ff007e3cb4eb06
Author:	 Tomasz Wasilczyk <tomkiewicz at cpw.pidgin.im>
Date:	 2013-03-20 20:51 +0100
Branch:	 soc.2008.masterpassword
URL: https://hg.pidgin.im/cpw/tomkiewicz/masterpassword/rev/21aa923e62ad

Description:

Fix use-after-free

diffstat:

 libpurple/keyring.c                    |   4 ++--
 libpurple/plugins/keyrings/kwallet.cpp |  24 ++++++++++++++++++++++--
 2 files changed, 24 insertions(+), 4 deletions(-)

diffs (87 lines):

diff --git a/libpurple/keyring.c b/libpurple/keyring.c
--- a/libpurple/keyring.c
+++ b/libpurple/keyring.c
@@ -468,12 +468,12 @@ purple_keyring_set_inuse_check_error_cb(
 				"/purple/keyring/active", purple_keyring_pref_cb, NULL);
 
 		} else {
+			purple_keyring_drop_passwords(tracker->old);
+
 			close = purple_keyring_get_close_keyring(tracker->old);
 			if (close != NULL)
 				close(&error);
 
-			purple_keyring_drop_passwords(tracker->old);
-
 			purple_debug_info("keyring", "Successfully changed keyring.\n");
 
 			if (tracker->cb != NULL)
diff --git a/libpurple/plugins/keyrings/kwallet.cpp b/libpurple/plugins/keyrings/kwallet.cpp
--- a/libpurple/plugins/keyrings/kwallet.cpp
+++ b/libpurple/plugins/keyrings/kwallet.cpp
@@ -90,6 +90,8 @@ class engine : private QObject, private 
 		bool failed;
 		bool closing;
 		bool externallyClosed;
+		bool busy;
+		bool closeAfterBusy;
 
 		KWallet::Wallet *wallet;
 
@@ -154,6 +156,8 @@ KWalletPlugin::engine::engine()
 	closing = false;
 	externallyClosed = false;
 	wallet = NULL;
+	busy = false;
+	closeAfterBusy = false;
 
 	reopenWallet();
 }
@@ -203,7 +207,8 @@ KWalletPlugin::engine::~engine()
 
 	delete wallet;
 
-	pinstance = NULL;
+	if (pinstance == this)
+		pinstance = NULL;
 }
 
 KWalletPlugin::engine *
@@ -221,7 +226,13 @@ KWalletPlugin::engine::closeInstance()
 		return;
 	if (pinstance->closing)
 		return;
-	delete pinstance;
+	if (pinstance->busy) {
+		purple_debug_misc("keyring-kwallet",
+			"current instance is busy, will be freed later\n");
+		pinstance->closeAfterBusy = true;
+	} else
+		delete pinstance;
+	pinstance = NULL;
 }
 
 void
@@ -275,6 +286,9 @@ KWalletPlugin::engine::queue(request *re
 void
 KWalletPlugin::engine::executeRequests()
 {
+	if (closing || busy)
+		return;
+	busy = true;
 	if (externallyClosed) {
 		reopenWallet();
 	} else if (connected || failed) {
@@ -289,6 +303,12 @@ KWalletPlugin::engine::executeRequests()
 	} else {
 		purple_debug_misc("keyring-kwallet", "not yet connected\n");
 	}
+	busy = false;
+	if (closeAfterBusy) {
+		purple_debug_misc("keyring-kwallet",
+			"instance freed after being busy\n");
+		delete this;
+	}
 }
 
 KWalletPlugin::save_request::save_request(PurpleAccount *acc, const char *pw,