Re: [PATCH] lib: fix memory error in notmuch_config_list_value

[Tomi Ollila <tomi.ollila@iki.fi>]

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)

Tomi

>  
>      /* 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
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> https://notmuchmail.org/mailman/listinfo/notmuch