Hi Eric,
On Mon, 15 Oct 2018, Eric Sunshine wrote:
> On Mon, Oct 15, 2018 at 5:47 AM Johannes Schindelin via GitGitGadget
> <gitgitgadget@xxxxxxxxx> wrote:
> > 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@xxxxxx>
> > ---
> > 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)?
Quite honestly, I think that this is so rare an instance (if it *can*
happen at all) that I simply don't care ;-)
Ciao,
Dscho
