From 438e3410002ddc256cc2ec5d0a11a46bc92764dd Mon Sep 17 00:00:00 2001 From: Johan Ouwerkerk Date: Wed, 1 Apr 2020 22:46:35 +0200 Subject: [PATCH] Avoid operations on a possibly expired account object. After triggering the removal of an account from storage in the UI, it may be possible for the UI not to fully reflect this change for a while yet. During this short time window, it is possible for an operation to occur on an already deleted account object which is a use-after-free bug. In particular signals from animations and timers in the QML UI might still trigger which causes a slot to be invoked that accesses the underlying account object. This change introduces a guard property called 'alive' which is flipped when the account removal operation is triggered. Slots are updated to check for the alive status of the UI before proceeding with other logic. --- src/contents/ui/AccountEntryView.qml | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/contents/ui/AccountEntryView.qml b/src/contents/ui/AccountEntryView.qml index 6c95e8c..ea0e842 100644 --- a/src/contents/ui/AccountEntryView.qml +++ b/src/contents/ui/AccountEntryView.qml @@ -19,13 +19,14 @@ Kirigami.SwipeListItem { property bool tokenAvailable: account && account.token && account.token.length > 0 property real healthIndicator: 0 + property bool alive: true property Kirigami.Action advanceCounter : Kirigami.Action { iconName: "go-next" // "view-refresh" text: "Next token" onTriggered: { // TODO convert to C++ helper, have proper logging? - if (account && account.isHotp) { + if (alive && account && account.isHotp) { account.advanceCounter(1); } // TODO warn if not @@ -37,7 +38,8 @@ Kirigami.SwipeListItem { text: "Delete account" onTriggered: { // TODO convert to C++ helper, have proper logging? - if (account) { + if (alive && account) { + alive = false; account.remove(); } // TODO warn if not @@ -70,7 +72,7 @@ Kirigami.SwipeListItem { interval: phase onTriggered: { // TODO convert to C++ helper, have proper logging? - if (account) { + if (alive && account) { if (account.isTotp) { timer.stop() timeoutIndicatorAnimation.stop(); @@ -117,7 +119,7 @@ Kirigami.SwipeListItem { onClicked: { // TODO convert to C++ helper, have proper logging? - if (tokenAvailable) { + if (alive && tokenAvailable) { Keysmith.copyToClipboard(account.token); } // TODO warn if not