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.
master
Johan Ouwerkerk 2020-04-01 22:46:35 +02:00
parent b451bd2556
commit 438e341000
1 changed files with 6 additions and 4 deletions

View File

@ -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