From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by arlo.cworth.org (Postfix) with ESMTP id 73E906DE022B for ; Sun, 24 Nov 2019 18:37:52 -0800 (PST) X-Virus-Scanned: Debian amavisd-new at cworth.org X-Spam-Flag: NO X-Spam-Score: -0.062 X-Spam-Level: X-Spam-Status: No, score=-0.062 tagged_above=-999 required=5 tests=[AWL=-0.061, SPF_PASS=-0.001] autolearn=disabled Received: from arlo.cworth.org ([127.0.0.1]) by localhost (arlo.cworth.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id pcsELsFHsxTr for ; Sun, 24 Nov 2019 18:37:50 -0800 (PST) Received: from fethera.tethera.net (fethera.tethera.net [198.245.60.197]) by arlo.cworth.org (Postfix) with ESMTPS id 070A36DE022A for ; Sun, 24 Nov 2019 18:37:49 -0800 (PST) Received: from remotemail by fethera.tethera.net with local (Exim 4.89) (envelope-from ) id 1iZ4G2-0005iw-W7; Sun, 24 Nov 2019 21:37:47 -0500 Received: (nullmailer pid 1235185 invoked by uid 1000); Mon, 25 Nov 2019 02:37:45 -0000 From: David Bremner To: notmuch@notmuchmail.org Subject: [PATCH] lib: fix memory error in notmuch_config_list_value Date: Sun, 24 Nov 2019 22:37:34 -0400 Message-Id: <20191125023734.1235130-1-david@tethera.net> X-Mailer: git-send-email 2.24.0 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-BeenThere: notmuch@notmuchmail.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "Use and development of the notmuch mail system." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 25 Nov 2019 02:37:52 -0000 The documentation for notmuch_config_list_key warns that that the returned value will be destroyed by the next call to notmuch_config_list_key, but it neglected to mention that calling notmuch_config_list_value would also destroy it (by calling notmuch_config_list_key). This is surprising, and caused a use after free bug in _setup_user_query_fields (first noticed by an OpenBSD porter, so kudos to the OpenBSD malloc implementation). This change fixes that use-after-free bug. --- lib/config.cc | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/lib/config.cc b/lib/config.cc index da71c16e..2cd8a0b6 100644 --- a/lib/config.cc +++ b/lib/config.cc @@ -150,13 +150,17 @@ notmuch_config_list_valid (notmuch_config_list_t *metadata) return true; } +static inline char * _key_from_iterator (notmuch_config_list_t *list) { + return talloc_strdup (list, (*list->iterator).c_str () + CONFIG_PREFIX.length ()); +} + const char * notmuch_config_list_key (notmuch_config_list_t *list) { if (list->current_key) talloc_free (list->current_key); - list->current_key = talloc_strdup (list, (*list->iterator).c_str () + CONFIG_PREFIX.length ()); + list->current_key = _key_from_iterator (list); return list->current_key; } @@ -166,7 +170,7 @@ notmuch_config_list_value (notmuch_config_list_t *list) { std::string strval; notmuch_status_t status; - const char *key = notmuch_config_list_key (list); + char *key = _key_from_iterator (list); /* TODO: better error reporting?? */ status = _metadata_value (list->notmuch, key, strval); @@ -177,6 +181,7 @@ notmuch_config_list_value (notmuch_config_list_t *list) talloc_free (list->current_val); list->current_val = talloc_strdup (list, strval.c_str ()); + talloc_free (key); return list->current_val; } -- 2.24.0