Re: [patch] credential-osxkeychain: Clear username_buffer before getting the converted C string.

> On 31 Jul 2024, at 08:42, Jeff King <peff@peff.net> wrote:
> 
> Hrm. Just looking at the code, here's a wild hypothesis: the problem
> could be not that the buffer is not NUL-terminated, but that after the
> NUL it contains junk, and we print that junk. That is, the code looks
> like this:
> 
>          /* If we can't get a CString pointer then
>           * we need to allocate our own buffer */
>          buffer_len = CFStringGetMaximumSizeForEncoding(
>                          CFStringGetLength(account_ref), ENCODING) + 1;
>          username_buf = xmalloc(buffer_len);
>          if (CFStringGetCString(account_ref,
>                                  username_buf,
>                                  buffer_len,
>                                  ENCODING)) {
>                  write_item("username", username_buf, buffer_len - 1);
>          }
> 
> So we asked the system for the _maximum_ size that the string could be
> (and added one for the NUL). Then we got the string, and we printed out
> the _whole_ buffer, not just the string up to the NUL. And your fix
> "works" because NULs end up getting ignored on the read side (or at
> least cause ruby not to complain about bogus utf8).
> 
> If that hypothesis is true, then the fix is more like:
> 
> diff --git a/contrib/credential/osxkeychain/git-credential-osxkeychain.c b/contrib/credential/osxkeychain/git-credential-osxkeychain.c
> index 6ce22a28ed..1c8310d7fe 100644
> --- a/contrib/credential/osxkeychain/git-credential-osxkeychain.c
> +++ b/contrib/credential/osxkeychain/git-credential-osxkeychain.c
> @@ -141,7 +141,7 @@ static void find_username_in_item(CFDictionaryRef item)
> 				username_buf,
> 				buffer_len,
> 				ENCODING)) {
> -		write_item("username", username_buf, buffer_len - 1);
> +		write_item("username", username_buf, strlen(username_buf));
> 	}
> 	free(username_buf);
> }