]> git.ipfire.org Git - thirdparty/git.git/commitdiff
credential/osxkeychain: respect NUL terminator in username
authorJeff King <peff@peff.net>
Thu, 1 Aug 2024 08:25:56 +0000 (04:25 -0400)
committerJunio C Hamano <gitster@pobox.com>
Thu, 1 Aug 2024 15:54:47 +0000 (08:54 -0700)
This patch fixes a case where git-credential-osxkeychain might output
uninitialized bytes to stdout.

We need to get the username string from a system API using
CFStringGetCString(). To do that, we get the max size for the string
from CFStringGetMaximumSizeForEncoding(), allocate a buffer based on
that, and then read into it. But then we print the entire buffer to
stdout, including the trailing NUL and any extra bytes which were not
needed. Instead, we should stop at the NUL.

This code comes from 9abe31f5f1 (osxkeychain: replace deprecated
SecKeychain API, 2024-02-17). The bug was probably overlooked back then
because this code is only used as a fallback when we can't get the
string via CFStringGetCStringPtr(). According to Apple's documentation:

  Whether or not this function returns a valid pointer or NULL depends
  on many factors, all of which depend on how the string was created and
  its properties.

So it's not clear how we could make a test for this, and we'll have to
rely on manually testing on a system that triggered the bug in the first
place.

Reported-by: Hong Jiang <ilford@gmail.com>
Signed-off-by: Jeff King <peff@peff.net>
Tested-by: Hong Jiang <ilford@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
contrib/credential/osxkeychain/git-credential-osxkeychain.c

index 6a40917b1ef866a3f9f5295dd887797df58af777..b5346005a46a36457f436f77048aa377e3f51a4e 100644 (file)
@@ -140,7 +140,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);
 }