From 4dbf8030411a1e2287be6e939336668e10f4a03a Mon Sep 17 00:00:00 2001 From: John Ralls Date: Mon, 19 Feb 2024 14:31:12 -0800 Subject: [PATCH] Fix two use-after-free issues found by address sanitizer. --- libgnucash/engine/Split.cpp | 8 +++++++- libgnucash/engine/Transaction.cpp | 9 ++++++--- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/libgnucash/engine/Split.cpp b/libgnucash/engine/Split.cpp index 2da0c51b9f..f481a57df0 100644 --- a/libgnucash/engine/Split.cpp +++ b/libgnucash/engine/Split.cpp @@ -706,7 +706,13 @@ xaccFreeSplit (Split *split) } CACHE_REMOVE(split->memo); CACHE_REMOVE(split->action); - + if (GNC_IS_ACCOUNT (split->acc) && !qof_instance_get_destroying (QOF_INSTANCE (split->acc))) + gnc_account_remove_split (split->acc, split); + if (GNC_IS_LOT (split->lot) && !qof_instance_get_destroying (QOF_INSTANCE (split->lot))) + gnc_lot_remove_split (split->lot, split); +/* We should do the same for split->parent but we might be getting + * called from xaccFreeTransactiob abd tgat would cause trouble. + */ /* Just in case someone looks up freed memory ... */ split->memo = (char *) 1; split->action = NULL; diff --git a/libgnucash/engine/Transaction.cpp b/libgnucash/engine/Transaction.cpp index 899adaa9d0..0ac8b346c4 100644 --- a/libgnucash/engine/Transaction.cpp +++ b/libgnucash/engine/Transaction.cpp @@ -1509,7 +1509,11 @@ do_destroy (Transaction *trans) done for the next split, then a split will still be on the split list after it has been freed. This can cause other parts of the code (e.g. in xaccSplitDestroy()) to reference the split after it has been freed. */ - for (node = trans->splits; node; node = node->next) + + auto splits = trans->splits; + trans->splits = NULL; + + for (node = splits; node; node = node->next) { Split *s = GNC_SPLIT(node->data); if (s && s->parent == trans) @@ -1517,7 +1521,7 @@ do_destroy (Transaction *trans) xaccSplitDestroy(s); } } - for (node = trans->splits; node; node = node->next) + for (node = splits; node; node = node->next) { Split *s = GNC_SPLIT(node->data); if (s && s->parent == trans) @@ -1526,7 +1530,6 @@ do_destroy (Transaction *trans) } } g_list_free (trans->splits); - trans->splits = NULL; xaccFreeTransaction (trans); }