]> git.feebdaed.xyz Git - 0xmirror/git-lfs.git/commit
docs,lfs,t: create new files on checkout and pull
authorChris Darroch <chrisd8088@github.com>
Fri, 16 May 2025 06:42:40 +0000 (23:42 -0700)
committerChris Darroch <chrisd8088@github.com>
Wed, 15 Oct 2025 05:04:28 +0000 (22:04 -0700)
commit5c11ffce9a4f095ff356bc781e2a031abb46c1a8
treed1786c543002f7982df6afce5db105a9dd2244a0
parent35728a6d0d488a5904cc9f03ac302680d0858583
docs,lfs,t: create new files on checkout and pull

Our "git lfs checkout" and "git lfs pull" commands, at present,
follow any extant symbolic links when they populate the current working
tree with files containing the content of Git LFS objects, even if
the symbolic links point to locations outside of the working tree.
This vulnerability has been assigned the identifier CVE-2025-26625.

In a previous commit we partially addressed this vulnerability by
adjusting the DecodePointerFromBlob() function in our "lfs" package to
check whether an irregular file or other directory entry exists at the
location where the commands intend to create or update a file.

While this change handles cases where a symbolic link already exists
the working tree before we try to create or update a file at the same
location, it does not entirely prevent TOCTOU (time-of-check/time-of-use)
races where a symbolic link might be created immediately after we check
for its existence and before we attempt to create or open a file.

One reason is that the "git lfs checkout" and "git lfs pull" commands
use the Create() function from the Go standard library's "os" package
to create or open the files they intend to populate with the contents
of Git LFS objects.  This function follows symbolic links when
determining whether it should create a new file or truncate an existing
one.  If the last segment of the path passed to the function is a
symbolic link, the link will be dereferenced, and a new file will be
created at the link's target path or, if a file already exists at that
target path, then that file will be opened and truncated.

Further, because the Create() function opens and truncates any existing
file it finds, if that file is hard-linked to one or more other
paths, then once the file is closed the new content our commands have
written into it will be visible through all of those paths, regardless
of whether they reside inside or outside the Git working tree.

Our "git lfs checkout" and "git lfs pull" commands have exhibited these
behaviours since they were first implemented in PR #527.  That PR
added a PointerSmudgeToFile() function to the "lfs" package, which was
later refactored by PR #2687 into the SmudgeToFile() method of the
GitFilter structure in the current version of our "lfs" package.  The
original PointerSmudgeToFile() function made use of the "os" package's
Create() function to create a new file or truncate an existing one,
and the contemporary SmudgeToFile() method follows suit.

For performance and compatibility reasons, Git does not try to
completely eliminate all TOCTOU races involving symbolic links, and
for similar reasons we do not expect to prevent every possible race
which might allow the Git LFS client to unintentionally write through
a symbolic link.  We do, though, intend to limit the chances of this
occurring as far as we reasonably can.

Therefore, to address the problems with symbolic and hard links described
above, we revise the SmudgeToFile() method so that it first removes any
existing file at the path it is given, and if that succeeds, then attempts
to atomically create a new file, reporting an error if that cannot be done
because a file or other directory entry already exists at the same path.

Specifically, we use the OpenFile() function from the "os" package
instead of the Create() function, and we pass both the O_CREATE and
O_EXCL flags to guarantee that the function either creates a new file
or returns an error.  Before calling OpenFile() we first call the
"os" package's Remove() function and report an error if it fails for
any reason other than that there is no file found at the given path.

This approach mirrors that taken by Git when it updates files in the
working tree.  In particular, when the "git checkout" command is
asked to update a specific pathspec (e.g., with a command such as
"git checkout -- file.txt"), the checkout_entry_ca() function first
calls the unlink(2) system call, and then the create_file() function
invokes the open(2) system call with the O_CREATE and O_EXCL flags:

  https://github.com/git/git/blob/cb96e1697ad6e54d11fc920c95f82977f8e438f8/entry.c#L552-L578
  https://github.com/git/git/blob/cb96e1697ad6e54d11fc920c95f82977f8e438f8/entry.c#L88-L89

Note that Git is actually more aggressive than the Git LFS client
in how it handles conflicting content when checking out specific
paths.  For instance, if it finds a directory in place of a file it
intends to write, its remove_subtree() function will be used to try
to recursively remove the directory and all of its contents.

By constrast, while our SmudgeToFile() method will now remove existing
files (whether regular or irregular), symbolic links, and empty
directories which conflict with the file it intends to create, the
function will will not remove non-empty directories.

Moreover, the SmudgeToFile() method will only take this action if one
of these types of directory entries has been created in the brief
time interval since the DecodePointerFromBlob() function was called,
since we use that function to determine whether to proceed to call
the SmudgeToFile() method.

The sole caller of the SmudgeToFile() method is the RunToPath()
method of the singleCheckout structure in our "commands" package,
which is used only by the "git lfs checkout" and "git lfs pull"
commands.  Except when "git lfs checkout" is called with a --to
option, the RunToPath() method is only called from the Run() method
of the same singleCheckout structure.  That method first invokes
the DecodePointerFromBlob() function, and proceeds to call the
SmudgeToFile() method only if no regular file was found, or if
a regular file was found and its contained a valid Git LFS pointer
whose ID matches that of the corresponding object.

For this reason, we are guaranteed that when the SmudgeToFile() method
is called, the path it is passed is either one provided by the user
with the --to option of the "git lfs checkout" command, or has just
been checked by the DecodePointerFromBlob() function.  In either case
we can be confident that it is reasonable to delete anything which now
exists at that location.  Note, too, that prior to the changes in this
commit, any regular file or file referenced by a final symbolic link
in the path would be truncated and overwritten regardless of its contents
by the SmudgeToFile() method, so removing any directory entry (except
for non-empty subdirectories) we find and creating a new file is not
substantially different in this respect.

However, there are several key advantages to our new approach.  First,
we can now be certain we will never dereference a final symbolic link
in the given path and write to the link's target.  Note, though, that
we do still traverse symbolic links when they are found in place of
directories in path segments other than the final segment.  We will
partially address this concern in a subsequent commit, with the same
caveats that apply to Git's handling of symbolic links in non-terminal
path segments.

Second, by always creating a new file we can be certain the content
we write will not be visible through hard links to an existing file.
We therefore add a pair of new tests to our t/t-checkout.sh and
t/t-pull.sh test suite which exercise the "git lfs checkout" and
"git lfs pull" commands and confirm that they replace existing files
with multiple hard links and effectively break those links.  Both of
our new tests use the assert_clean_status() test helper function to
confirm that the "git lfs checkout" and "git lfs pull" commands continue
to update the Git index entries for any Git LFS files they recreate in
the working tree.

We also expand the checks performed by the "checkout: conflicts" test
in our t/t-checkout.sh test script to check that symbolic links as well
as hard links are broken by our changes to the SmudgeToFile() method.
We are able to use this test for this purpose because it runs the
"git lfs checkout" command with the --to option, which means the
Run() method of the singleCheckout structure is not used and the
RunToPath() method is called directly.  In turn, that implies that
the DecodePointerFromBlob() function is never invoked, so the command
does not simply detect the symbolic link in that function and therefore
skip making a call to the SmudgeToFile() method, as occurs in our
"checkout: skip file symlink conflicts" and "pull: skip file symlink
conflicts" tests.  Instead, the RunToPath() method calls the
SmudgeToFile() method, which then removes the symbolic link and
creates a new file in its place.  Hence we can use this test to confirm
that our changes are effective in breaking symbolic links as well as
hard links.

And third, our new approach means we can eliminate two calls to the
"os" package's Chmod() function, which were added to the SmudgeToFile()
method in commit 686bda3722f12293f345240532f666b6a0961bb2 of PR #3120
in order to handle pointer files to which our "lockable" Git attribute
applies, but which the user has not yet locked, and so the pointer files
have read-only permissions we want to retain while also replacing the
file's contents with the corresponding Git LFS object data.

We do not need to call the Chmod() function before invoking the
Remove() function, because that function should be able to delete any
existing file, even one with read-only permissions, so long as the
parent directory permits changes to its list of entries.

Note that our previous implementation might succeed even if the
parent directory did not allow changes to its list of entries, and
our new implementation will not.  This does imply a partial change
in the behaviour of the Git LFS client when directories in the
working tree are themselves marked read-only.  However, neither our
old or new implementations could succeed in creating new files within
such directories.  Moreover, we expect Git working trees to normally
have read-write directory permissions, since many regular Git commands
will not function otherwise.  We therefore consider the altered
behaviour of the Git LFS client to be an acceptable change given
that it will remediate several security concerns.

We also do not need to call the Chmod() function at the end of the
SmudgeToFile() method, because we instead pass the file permissions
we want directly to the OpenFile() function.

In the case where an existing file is found, prior to our deletion of
that file, we read its permissions with the Lstat() function of the
"os" package, and then pass those permissions to the OpenFile()
function.  If a symbolic link or some other type of directory entry
is found, though, we ignore its permissions and use a default setting
of 0666 instead.  (On Unix systems, the current "umask" setting will
then be applied to whatever permissions we pass to the OpenFile()
function.)

While the use of a default permissions mode of 0666 matches that used
by the Create() function of the "os" package, and so aligns with the
legacy behaviour of the Git LFS client, this is not actually the ideal
implementation.  Rather, we should respect the mode defined for the
file in Git, which may have the executable mode set.  For now, though,
we leave this as improvement for a future PR, and just include a
comment to remind us of this oversight in our implementation.

Finally, because the "git lfs checkout" command will now attempt to
remove and replace the file or other directory entry it finds at the
path supplied with the --to option, we update our git-lfs-checkout(1)
manual page to reflect this new behaviour.
docs/man/git-lfs-checkout.adoc
lfs/gitfilter_smudge.go
t/t-checkout.sh
t/t-pull.sh