CVE-2024-53263

@@ -58,21 +58,25 @@ func (c Creds) IsMultistage() bool {
 	return slices.Contains([]string{"1", "true"}, FirstEntryForKey(c, "continue"))
 }
 
-func (c Creds) buffer() *bytes.Buffer {
+func (c Creds) buffer() (*bytes.Buffer, error) {
 	buf := new(bytes.Buffer)
 
 	buf.Write([]byte("capability[]=authtype\n"))
 	buf.Write([]byte("capability[]=state\n"))
 	for k, v := range c {
 		for _, item := range v {
+			if strings.Contains(item, "\n") {
+				return nil, errors.Errorf(tr.Tr.Get("credential value for %s contains newline: %q", k, item))
+			}
+
 			buf.Write([]byte(k))
 			buf.Write([]byte("="))
 			buf.Write([]byte(item))
 			buf.Write([]byte("\n"))
 		}
 	}
 
-	return buf
+	return buf, nil
 }
 
 type CredentialHelperContext struct {
@@ -338,7 +342,10 @@ func (h *commandCredentialHelper) exec(subcommand string, input Creds) (Creds, e
 	if err != nil {
 		return nil, errors.New(tr.Tr.Get("failed to find `git credential %s`: %v", subcommand, err))
 	}
-	cmd.Stdin = input.buffer()
+	cmd.Stdin, err = input.buffer()
+	if err != nil {
+		return nil, errors.New(tr.Tr.Get("invalid input to `git credential %s`: %v", subcommand, err))
+	}
 	cmd.Stdout = output
 	/*
 	   There is a reason we don't read from stderr here:

@@ -25,7 +25,8 @@ func TestCredsBufferFormat(t *testing.T) {
 
 	expected := []string{"capability[]=authtype\n", "capability[]=state\n"}
 
-	buf := creds.buffer()
+	buf, err := creds.buffer()
+	assert.NoError(t, err)
 	assertCredsLinesMatch(t, expected, buf)
 
 	creds["protocol"] = []string{"https"}
@@ -34,7 +35,8 @@ func TestCredsBufferFormat(t *testing.T) {
 	expectedPrefix := strings.Join(expected, "")
 	expected = append(expected, "protocol=https\n", "host=example.com\n")
 
-	buf = creds.buffer()
+	buf, err = creds.buffer()
+	assert.NoError(t, err)
 	assert.True(t, strings.HasPrefix(buf.String(), expectedPrefix))
 	assertCredsLinesMatch(t, expected, buf)
 
@@ -43,11 +45,24 @@ func TestCredsBufferFormat(t *testing.T) {
 	expected = append(expected, "wwwauth[]=Basic realm=test\n")
 	expected = append(expected, "wwwauth[]=Negotiate\n")
 
-	buf = creds.buffer()
+	buf, err = creds.buffer()
+	assert.NoError(t, err)
 	assert.True(t, strings.HasPrefix(buf.String(), expectedPrefix))
 	assertCredsLinesMatch(t, expected, buf)
 }
 
+func TestCredsBufferProtect(t *testing.T) {
+	creds := make(Creds)
+
+	// Always disallow LF characters
+	creds["protocol"] = []string{"https"}
+	creds["host"] = []string{"one.example.com\nhost=two.example.com"}
+
+	buf, err := creds.buffer()
+	assert.Error(t, err)
+	assert.Nil(t, buf)
+}
+
 type testCredHelper struct {
 	fillErr    error
 	approveErr error

@@ -0,0 +1,46 @@
+#!/usr/bin/env bash
+
+. "$(dirname "$0")/testlib.sh"
+
+ensure_git_version_isnt $VERSION_LOWER "2.3.0"
+
+export CREDSDIR="$REMOTEDIR/creds-credentials-protect"
+setup_creds
+
+# Copy the default record file for the test credential helper to match the
+# hostname used in the Git LFS configurations of the tests.
+cp "$CREDSDIR/127.0.0.1" "$CREDSDIR/localhost"
+
+begin_test "credentials rejected with line feed"
+(
+  set -e
+
+  reponame="protect-linefeed"
+  setup_remote_repo "$reponame"
+  clone_repo "$reponame" "$reponame"
+
+  contents="a"
+  contents_oid=$(calc_oid "$contents")
+
+  git lfs track "*.dat"
+  printf "%s" "$contents" >a.dat
+  git add .gitattributes a.dat
+  git commit -m "add a.dat"
+
+  # Using localhost instead of 127.0.0.1 in the LFS API URL ensures this URL
+  # is used when filling credentials rather than the Git remote URL, which
+  # would otherwise be used since it would have the same scheme and hostname.
+  gitserver="$(echo "$GITSERVER" | sed 's/127\.0\.0\.1/localhost/')"
+  testreponame="test%0a$reponame"
+  git config lfs.url "$gitserver/$testreponame.git/info/lfs"
+
+  GIT_TRACE=1 git lfs push origin main 2>&1 | tee push.log
+  if [ "0" -eq "${PIPESTATUS[0]}" ]; then
+    echo >&2 "fatal: expected 'git lfs push' to fail ..."
+    exit 1
+  fi
+  grep "batch response: Git credentials for $gitserver.* not found" push.log
+  grep "credential value for path contains newline" push.log
+  refute_server_object "$testreponame" "$contents_oid"
+)
+end_test