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;
>>  }
>>  
>> +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);
>
> 2 spaces, otherwise looks good (on paper)

I concur with Tomi.  Please clean up the 2 spaces, and merge!

  --dkg