[PATCH 3/7] contrib/credential: avoid fixed-size buffer in osxkeychain

From: Taylor Blau <me@ttaylorr.com>
Date: May 1, 2023, 15:53 UTC
Message-ID: <301bc27a5b4500ff6681bbf5a113d763b8f0b4c9.1682956419.git.me@ttaylorr.com>
In-Reply-To: <cover.1682956419.git.me@ttaylorr.com>

The macOS Keychain-based credential helper reads the newline-delimited protocol stream one line at a time by repeatedly calling fgets() into a fixed-size buffer, and is thus affected by the vulnerability described in the previous commit.

To mitigate this attack, avoid using a fixed-size buffer, and instead rely on getline() to allocate a buffer as large as necessary to fit the entire content of the line, preventing any protocol injection.

We solved a similar problem in a5bb10fd5e (config: avoid fixed-sized buffer when renaming/deleting a section, 2023-04-06) by switching to strbuf_getline(). We can't do that here because the contrib helpers do not link with the rest of Git, and so can't use a strbuf. But we can use the system getline() directly, which works similarly.

In most parts of Git we don't assume that every platform has getline(). But this helper is run only on OS X, and that platform added support in 10.7 ("Lion") which was released in 2011.

Tested-by: Taylor Blau <me@ttaylorr.com>
Co-authored-by: Jeff King <peff@peff.net>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
 .../osxkeychain/git-credential-osxkeychain.c           | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/contrib/credential/osxkeychain/git-credential-osxkeychain.c b/contrib/credential/osxkeychain/git-credential-osxkeychain.c
index e29cc28779..5f2e5f16c8 100644
--- a/contrib/credential/osxkeychain/git-credential-osxkeychain.c
+++ b/contrib/credential/osxkeychain/git-credential-osxkeychain.c
@@ -113,14 +113,16 @@ static void add_internet_password(void)
 
 static void read_credential(void)
 {
-	char buf[1024];
+	char *buf = NULL;
+	size_t alloc;
+	ssize_t line_len;
 
-	while (fgets(buf, sizeof(buf), stdin)) {
+	while ((line_len = getline(&buf, &alloc, stdin)) > 0) {
 		char *v;
 
 		if (!strcmp(buf, "\n"))
 			break;
-		buf[strlen(buf)-1] = '\0';
+		buf[line_len-1] = '\0';
 
 		v = strchr(buf, '=');
 		if (!v)
@@ -165,6 +167,8 @@ static void read_credential(void)
 		 * learn new lines, and the helpers are updated to match.
 		 */
 	}
+
+	free(buf);
 }
 
 int main(int argc, const char **argv)

-- 
2.40.1.452.gb3cd41c833