From 4e649d000b9d3764aea98cb0e1120947d7f76f3d Mon Sep 17 00:00:00 2001 From: David Bremner Date: Wed, 22 Feb 2017 06:17:47 -0400 Subject: [PATCH 1/1] lib: fix g_hash_table related read-after-free bug The two g_hash_table functions (insert, add) have different behaviour with respect to existing keys. g_hash_table_insert frees the new key, while g_hash_table_add (which is really g_hash_table_replace in disguise) frees the existing key. With this change 'ref' is live until the end of the function (assuming single-threaded access to 'hash'). We can't guarantee it will continue to be live in the future (i.e. there may be a future key duplication) so we copy it with the allocation context passed to parse_references (in practice this is the notmuch_message_t object whose parents we are finding). Thanks to Tomi for the simpler approach to the problem based on reading the fine glib manual. --- lib/database.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/database.cc b/lib/database.cc index f0bfe566..eddb780c 100644 --- a/lib/database.cc +++ b/lib/database.cc @@ -652,7 +652,7 @@ parse_references (void *ctx, ref = _parse_message_id (ctx, refs, &refs); if (ref && strcmp (ref, message_id)) { - g_hash_table_insert (hash, ref, NULL); + g_hash_table_add (hash, ref); last_ref = ref; } } @@ -661,7 +661,7 @@ parse_references (void *ctx, * reference to the database. We should avoid making a message * its own parent, thus the above check. */ - return last_ref; + return talloc_strdup(ctx, last_ref); } notmuch_status_t -- 2.43.0