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 97A6F6DE0EB5 for ; Mon, 9 Dec 2019 14:39:06 -0800 (PST) Authentication-Results: arlo.cworth.org; dkim=permerror (0-bit key) header.d=fifthhorseman.net header.i=@fifthhorseman.net header.b="l/2LeCeZ"; dkim=pass (2048-bit key; unprotected) header.d=fifthhorseman.net header.i=@fifthhorseman.net header.b="hzEbm5rQ"; dkim-atps=neutral X-Virus-Scanned: Debian amavisd-new at cworth.org X-Spam-Flag: NO X-Spam-Score: -1.521 X-Spam-Level: X-Spam-Status: No, score=-1.521 tagged_above=-999 required=5 tests=[AWL=0.980, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_MED=-2.3, 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 V9spRoKMC9kF for ; Mon, 9 Dec 2019 14:39:05 -0800 (PST) Received: from che.mayfirst.org (che.mayfirst.org [162.247.75.118]) by arlo.cworth.org (Postfix) with ESMTPS id CA7F56DE0EC6 for ; Mon, 9 Dec 2019 14:39:04 -0800 (PST) DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/simple; d=fifthhorseman.net; i=@fifthhorseman.net; q=dns/txt; s=2019; t=1575931143; h=from : to : subject : in-reply-to : references : date : message-id : mime-version : content-type : from; bh=DLL2pe5n6QweNmPZ3lHFRTS+G/2hHSuciC6rbn9Wweo=; b=l/2LeCeZRV3ZPB+bdbMnZc0nQ1lCMwVOn7KsJA3sjvrpz0dlACyXQ2bz Ygs0gVezksji+XJf1UAtMOigA1bqCA== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=fifthhorseman.net; i=@fifthhorseman.net; q=dns/txt; s=2019rsa; t=1575931143; h=from : to : subject : in-reply-to : references : date : message-id : mime-version : content-type : from; bh=DLL2pe5n6QweNmPZ3lHFRTS+G/2hHSuciC6rbn9Wweo=; b=hzEbm5rQvWStbr6fZKN7DO7d1clf0/d9UK4VZ5XIB/HWJwniGu1UE7ZC dQM3cbnBMYQmzMxGBS00dQgw5qekWDkaMmT0ZgJiY24V6d4zEjjf2vjop4 KueF3+ATtlUCQO6QmHRKmkfzyJaJ/lrTVkHTxCdzrh6kUBmkCSinuiRE8z ITIq5nsuOhb+d9usiUfi0wpasvndOTmwteXFeDkN2tnyr/YeO6JL6v6G4R FkaZ643HM0nT8Z4Jcq1+ZeBbO0GzxbMEs0Wo/6nrmCfDmJzh0GwrBuVzZJ e1fugD/wr+Mm8wXEr1F3PrCnT7bPu+N5xxnHb4LIME1c3w/vsgQZCg== Received: from fifthhorseman.net (unknown [38.109.115.130]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by che.mayfirst.org (Postfix) with ESMTPSA id 6F258F9A5; Mon, 9 Dec 2019 17:39:02 -0500 (EST) Received: by fifthhorseman.net (Postfix, from userid 1000) id 14EAE203F1; Mon, 9 Dec 2019 15:31:43 -0500 (EST) From: Daniel Kahn Gillmor To: Tomi Ollila , David Bremner , notmuch@notmuchmail.org Subject: Re: [PATCH] lib: fix memory error in notmuch_config_list_value In-Reply-To: References: <20191125023734.1235130-1-david@tethera.net> Autocrypt: addr=dkg@fifthhorseman.net; prefer-encrypt=mutual; keydata= mDMEXEK/AhYJKwYBBAHaRw8BAQdAr/gSROcn+6m8ijTN0DV9AahoHGafy52RRkhCZVwxhEe0K0Rh bmllbCBLYWhuIEdpbGxtb3IgPGRrZ0BmaWZ0aGhvcnNlbWFuLm5ldD6ImQQTFggAQQIbAQUJA8Jn AAULCQgHAgYVCgkICwIEFgIDAQIeAQIXgBYhBMS8Lds4zOlkhevpwvIGkReQOOXGBQJcQsbzAhkB AAoJEPIGkReQOOXG4fkBAO1joRxqAZY57PjdzGieXLpluk9RkWa3ufkt3YUVEpH/AP9c+pgIxtyW +FwMQRjlqljuj8amdN4zuEqaCy4hhz/1DbgzBFxCv4sWCSsGAQQB2kcPAQEHQERSZxSPmgtdw6nN u7uxY7bzb9TnPrGAOp9kClBLRwGfiPUEGBYIACYWIQTEvC3bOMzpZIXr6cLyBpEXkDjlxgUCXEK/ iwIbAgUJAeEzgACBCRDyBpEXkDjlxnYgBBkWCAAdFiEEyQ5tNiAKG5IqFQnndhgZZSmuX/gFAlxC v4sACgkQdhgZZSmuX/iVWgD/fCU4ONzgy8w8UCHGmrmIZfDvdhg512NIBfx+Mz9ls5kA/Rq97vz4 z48MFuBdCuu0W/fVqVjnY7LN5n+CQJwGC0MIA7QA/RyY7Sz2gFIOcrns0RpoHr+3WI+won3xCD8+ sVXSHZvCAP98HCjDnw/b0lGuCR7coTXKLIM44/LFWgXAdZjm1wjODbg4BFxCv50SCisGAQQBl1UB BQEBB0BG4iXnHX/fs35NWKMWQTQoRI7oiAUt0wJHFFJbomxXbAMBCAeIfgQYFggAJhYhBMS8Lds4 zOlkhevpwvIGkReQOOXGBQJcQr+dAhsMBQkB4TOAAAoJEPIGkReQOOXGe/cBAPlek5d9xzcXUn/D kY6jKmxe26CTws3ZkbK6Aa5Ey/qKAP0VuPQSCRxA7RKfcB/XrEphfUFkraL06Xn/xGwJ+D0hCw== Date: Mon, 09 Dec 2019 15:31:42 -0500 Message-ID: <878snl8clt.fsf@fifthhorseman.net> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha256; protocol="application/pgp-signature" 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, 09 Dec 2019 22:39:06 -0000 --=-=-= Content-Type: text/plain Content-Transfer-Encoding: quoted-printable On Mon 2019-11-25 19:21:24 +0200, Tomi Ollila wrote: > On Sun, Nov 24 2019, David Bremner wrote: > >> 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; >> } >>=20=20 >> +static inline char * _key_from_iterator (notmuch_config_list_t *list) { >> + return talloc_strdup (list, (*list->iterator).c_str () + CONFIG_PRE= FIX.length ()); >> +} >> + >> const char * >> notmuch_config_list_key (notmuch_config_list_t *list) >> { >> if (list->current_key) >> talloc_free (list->current_key); >>=20=20 >> - list->current_key =3D talloc_strdup (list, (*list->iterator).c_str = () + CONFIG_PREFIX.length ()); >> + list->current_key =3D _key_from_iterator (list); >>=20=20 >> return list->current_key; >> } >> @@ -166,7 +170,7 @@ notmuch_config_list_value (notmuch_config_list_t *li= st) >> { >> std::string strval; >> notmuch_status_t status; >> - const char *key =3D notmuch_config_list_key (list); >> + char *key =3D _key_from_iterator (list); > > 2 spaces, otherwise looks good (on paper) I concur with Tomi. Please clean up the 2 spaces, and merge! --dkg --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iHUEARYIAB0WIQTJDm02IAobkioVCed2GBllKa5f+AUCXe6vLgAKCRB2GBllKa5f +CFvAP4q5jvJ8mAqd+H4eG+dbozImDQjJSQYbtNJ3C157zl32QEAixD2QT/kcuFS cMyW2M9LOF+1RNgJXLxwWUFrHeL+ZAU= =bg+W -----END PGP SIGNATURE----- --=-=-=--