From 7bd97f15d0fdbd53e752c173d616bf032810e57b Mon Sep 17 00:00:00 2001 From: John Ralls Date: Tue, 20 Feb 2024 15:10:05 -0800 Subject: [PATCH] Fix transaction delete use-after-free, take 2. The problem with take 1 was that the duplicate split uses the same lot and account pointers without adding itself to those lists, causing checks in unit tests to fail. --- libgnucash/engine/Split.cpp | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/libgnucash/engine/Split.cpp b/libgnucash/engine/Split.cpp index 2da0c51b9f..19797df438 100644 --- a/libgnucash/engine/Split.cpp +++ b/libgnucash/engine/Split.cpp @@ -707,6 +707,19 @@ xaccFreeSplit (Split *split) CACHE_REMOVE(split->memo); CACHE_REMOVE(split->action); + if (split->inst.e_type) /* Don't do this for dupe splits. */ + { + /* gnc_lot_remove_split needs the account, so do it first. */ + if (GNC_IS_LOT (split->lot) && !qof_instance_get_destroying (QOF_INSTANCE (split->lot))) + gnc_lot_remove_split (split->lot, split); + if (GNC_IS_ACCOUNT (split->acc) + && !qof_instance_get_destroying (QOF_INSTANCE (split->acc))) + gnc_account_remove_split (split->acc, split); + /* We should do the same for split->parent but we might be getting + * called from xaccFreeTransaction and that would cause trouble. + */ + } + /* Just in case someone looks up freed memory ... */ split->memo = (char *) 1; split->action = NULL;