creds/creds.go | 14 11 + 3 - 0 !
creds/creds_test.go | 12 12 + 0 - 0 !
t/t-credentials-protect.sh | 46 46 + 0 - 0 !
3 files changed, 69 insertions(+), 3 deletions(-)

 creds/creds.go: reject lf bytes in credential data

As reported in CVE-2024-53263, when Git LFS writes credential request
keys and values to a Git credential helper via the git-credential(1)
command, it does not remove any line feed (LF) control characters
included in the values, which may allow a malicious server to insert
spurious keys and values into the credential request.

Because the "git credential fill" command receives these values as its
input, it is not able to disambiguate the line feed characters we add
at the end of each input line from those which are the result of
decoding URL-encoded characters.

When Git processes a URL and passes values from that URL to a credential
helper, any line feed characters which have been decoded are detected by
a check in the credential_write_item() function of the credential.c
source file and cause Git to exit with an error.  This check was added
in commit git/git@9a6bbee8006c24b46a85d29e7b38cfa79e9ab21b and released
in Git version 2.17.4 (and other patch releases) to address the
vulnerability reported in CVE-2020-5260:

  https://github.com/git/git/blob/v2.47.1/credential.c#L391-L392

Given that the Git security issue in CVE-2020-5260 has the same cause
and effect as the one reported for Git LFS in CVE-2024-53263, we adopt
the same approach as Git to resolve the latter issue now.  To do this
we revise the buffer() method of the Creds structure in our "creds"
package (which we refactored from a bufferCreds() function in a prior
commit in this PR) so it returns an error if any of the values it
processes have an embedded line feed character.  We then alter the sole
caller of this method, the exec() method of our commandCredentialHelper
structure in the same package, to detect and return any error from the
buffer() method.

To verify these changes we add a TestCredsBufferProtect() test function,
which checks that the buffer() method returns an error when one of the
values it is given contains a line feed character.

We also create a new t/t-credentials-protect.sh test script with one
initial test named "credentials rejected with line feed".  This test
confirms that an error is reported and no Git LFS object is pushed if a
value which would be passed to a "git credential fill" command contains
a bare line feed character decoded from a URL-encoded line feed character
found in a URL.

We use the "localhost" hostname in this URL rather than the 127.0.0.1
address on which our lfstest-gitserver utility listens for requests
because otherwise Git LFS will detect that the scheme and the host
subcomponent of the URL for the Git LFS API match those of the URL of
the current Git remote, and will then use the Git remote's URL when
querying for credentials.  As that URL does not contain a URL-encoded
line feed character, our test would not achieve its purpose, so we
instead use the "localhost" alias in the test's Git LFS configuration.

As this test makes use of the "localhost" hostname, we provide a
credential record file associated with that hostname for our
git-credential-lfstest helper to find and use.  To establish this file
we copy the record file associated with the 127.0.0.1 address, as
that file has already been created by the setup_creds() function from
our t/testhelpers.sh library.

This test does not actually depend on the existence of a credential
record file for the "localhost" hostname, as we expect the credential
helper to never be consulted due to the presence of a line feed character
in the values that would be sent to it.  However, we expect to add
another test to our new test script in a subsequent commit in this PR,
and that test will depend on the credential helper finding a record file
for the "localhost" hostname.

The presence of the record file also ensures that if in the future a
regression should be introduced such that values containing a bare
line feed character are passed to the credential helper, our test would
not continue to pass.  If we did not establish a record file, and a
regression caused the bare line feed character to be ignored, the
"git lfs push" command would still fail because it would not find
credentials for the "localhost" hostname, and so no object would be
pushed, as the test expects, and thus the test would continue to pass.

Instead, in the case of a regression we want the "git lfs push" command
to succeed, and then the subsequent call to the refute_server_object()
function to fail and cause the test to fail.  We therefore create a
valid credential record file even though it is not used at present.

Some of the tests in the existing t/t-credentials.sh test script depend
on a credential record file for the "localhost" hostname not existing,
though, which is why we define our new test in a new, separate test
script.  We can then set a unique path for the CREDSDIR environment
variable in that script, which guarantees that any modifications the
script makes to the set of credential record files do not affect any
other script's tests.

Reported-by: RyotaK from Flatt Security Inc.