Re: [PATCH 3/3] mingw: use domain information for default email

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Oct 15, 2018, 14:41 UTC
Message-ID: <CAPig+cRaBRP9f3sR8suZxC+Y2_UZaBMbMcpjsRUY+mGN+nyXkQ@mail.gmail.com>
In-Reply-To: <271a5f57cef00d24ee33e99839cf3890e561efe7.1539596822.git.gitgitgadget@gmail.com>

On Mon, Oct 15, 2018 at 5:47 AM Johannes Schindelin via GitGitGadget <gitgitgadget@gmail.com> wrote:

Show 16 quoted lines

> When a user is registered in a Windows domain, it is really easy to
> obtain the email address. So let's do that.
> [...]
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
> diff --git a/compat/mingw.c b/compat/mingw.c
> @@ -1826,6 +1826,11 @@ static char *get_extended_user_info(enum EXTENDED_NAME_FORMAT type)
> +char *mingw_query_user_email(void)
> +{
> +       return get_extended_user_info(NameUserPrincipal);
> +}
> diff --git a/ident.c b/ident.c
> @@ -168,6 +168,9 @@ const char *ident_default_email(void)
> +               } else if ((email = query_user_email()) && email[0]) {
> +                       strbuf_addstr(&git_default_email, email);
> +                       free((char *)email);

If query_user_email(), which calls get_extended_user_info(), returns NULL, then we do nothing (and don't worry about freeing the result). However, if query_user_email() returns a zero-length allocated string, then this conditional will leak that string (due to the email[0] check). From patch 2/3, we see in get_extended_user_info():

+static char *get_extended_user_info(enum EXTENDED_NAME_FORMAT type)
+{
+       if (GetUserNameExW(type, wbuffer, &len)) {
+               char *converted = xmalloc((len *= 3));
+               if (xwcstoutf(converted, wbuffer, len) >= 0)
+                       return converted;

that it may possibly return a zero-length string (due to the ">=0"). Do we care about this potential leak (or is GetUserNameExW() guaranteed never to return such a string)?