diff --git a/creds/creds.go b/creds/creds.go index baa314666b..016bfafda4 100644 --- a/creds/creds.go +++ b/creds/creds.go @@ -58,7 +58,7 @@ func (c Creds) IsMultistage() bool { return slices.Contains([]string{"1", "true"}, FirstEntryForKey(c, "continue")) } -func (c Creds) buffer() (*bytes.Buffer, error) { +func (c Creds) buffer(protectProtocol bool) (*bytes.Buffer, error) { buf := new(bytes.Buffer) buf.Write([]byte("capability[]=authtype\n")) @@ -68,6 +68,9 @@ func (c Creds) buffer() (*bytes.Buffer, error) { if strings.Contains(item, "\n") { return nil, errors.Errorf(tr.Tr.Get("credential value for %s contains newline: %q", k, item)) } + if protectProtocol && strings.Contains(item, "\r") { + return nil, errors.Errorf(tr.Tr.Get("credential value for %s contains carriage return: %q\nIf this is intended, set `credential.protectProtocol=false`", k, item)) + } buf.Write([]byte(k)) buf.Write([]byte("=")) @@ -172,6 +175,9 @@ func (ctxt *CredentialHelperContext) GetCredentialHelper(helper CredentialHelper helpers = append(helpers, ctxt.askpassCredHelper) } } + + ctxt.commandCredHelper.protectProtocol = ctxt.urlConfig.Bool("credential", rawurl, "protectProtocol", true) + return CredentialHelperWrapper{CredentialHelper: NewCredentialHelpers(append(helpers, ctxt.commandCredHelper)), Input: input, Url: u} } @@ -311,7 +317,8 @@ func (a *AskPassCredentialHelper) args(prompt string) []string { } type commandCredentialHelper struct { - SkipPrompt bool + SkipPrompt bool + protectProtocol bool } func (h *commandCredentialHelper) Fill(creds Creds) (Creds, error) { @@ -342,7 +349,7 @@ 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, err = input.buffer() + cmd.Stdin, err = input.buffer(h.protectProtocol) if err != nil { return nil, errors.New(tr.Tr.Get("invalid input to `git credential %s`: %v", subcommand, err)) } diff --git a/creds/creds_test.go b/creds/creds_test.go index 862b40a78d..bdf522e573 100644 --- a/creds/creds_test.go +++ b/creds/creds_test.go @@ -25,7 +25,7 @@ func TestCredsBufferFormat(t *testing.T) { expected := []string{"capability[]=authtype\n", "capability[]=state\n"} - buf, err := creds.buffer() + buf, err := creds.buffer(true) assert.NoError(t, err) assertCredsLinesMatch(t, expected, buf) @@ -35,7 +35,7 @@ func TestCredsBufferFormat(t *testing.T) { expectedPrefix := strings.Join(expected, "") expected = append(expected, "protocol=https\n", "host=example.com\n") - buf, err = creds.buffer() + buf, err = creds.buffer(true) assert.NoError(t, err) assert.True(t, strings.HasPrefix(buf.String(), expectedPrefix)) assertCredsLinesMatch(t, expected, buf) @@ -45,7 +45,7 @@ func TestCredsBufferFormat(t *testing.T) { expected = append(expected, "wwwauth[]=Basic realm=test\n") expected = append(expected, "wwwauth[]=Negotiate\n") - buf, err = creds.buffer() + buf, err = creds.buffer(true) assert.NoError(t, err) assert.True(t, strings.HasPrefix(buf.String(), expectedPrefix)) assertCredsLinesMatch(t, expected, buf) @@ -58,7 +58,29 @@ func TestCredsBufferProtect(t *testing.T) { creds["protocol"] = []string{"https"} creds["host"] = []string{"one.example.com\nhost=two.example.com"} - buf, err := creds.buffer() + buf, err := creds.buffer(false) + assert.Error(t, err) + assert.Nil(t, buf) + + buf, err = creds.buffer(true) + assert.Error(t, err) + assert.Nil(t, buf) + + // Disallow CR characters unless protocol protection disabled + creds["host"] = []string{"one.example.com\rhost=two.example.com"} + + expected := []string{ + "capability[]=authtype\n", + "capability[]=state\n", + "protocol=https\n", + "host=one.example.com\rhost=two.example.com\n", + } + + buf, err = creds.buffer(false) + assert.NoError(t, err) + assertCredsLinesMatch(t, expected, buf) + + buf, err = creds.buffer(true) assert.Error(t, err) assert.Nil(t, buf) } diff --git a/t/cmd/lfstest-gitserver.go b/t/cmd/lfstest-gitserver.go index 1d2ef84a0d..267bce0e7e 100644 --- a/t/cmd/lfstest-gitserver.go +++ b/t/cmd/lfstest-gitserver.go @@ -26,6 +26,7 @@ import ( "net/http" "net/http/httptest" "net/textproto" + "net/url" "os" "os/exec" "regexp" @@ -257,6 +258,7 @@ func lfsHandler(w http.ResponseWriter, r *http.Request, id string) { } func lfsUrl(repo, oid string, redirect bool) string { + repo = url.QueryEscape(repo) if redirect { return server.URL + "/redirect307/objects/" + oid + "?r=" + repo } diff --git a/t/t-credentials-protect.sh b/t/t-credentials-protect.sh index 89bbd8761b..0807640253 100755 --- a/t/t-credentials-protect.sh +++ b/t/t-credentials-protect.sh @@ -42,5 +42,60 @@ begin_test "credentials rejected with line feed" 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" + + git config credential.protectProtocol false + + 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 + +begin_test "credentials rejected with carriage return" +( + set -e + + reponame="protect-return" + 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%0d$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 carriage return" push.log + refute_server_object "$testreponame" "$contents_oid" + + git config credential.protectProtocol false + + git lfs push origin main 2>&1 | tee push.log + if [ "0" -ne "${PIPESTATUS[0]}" ]; then + echo >&2 "fatal: expected 'git lfs push' to succeed ..." + exit 1 + fi + [ $(grep -c "Uploading LFS objects: 100% (1/1)" push.log) -eq 1 ] + assert_server_object "$testreponame" "$contents_oid" ) end_test