<feed xmlns='http://www.w3.org/2005/Atom'>
<title>git/symlinks.c, branch jch</title>
<subtitle>Mirror of https://git.kernel.org/pub/scm/git/git.git/
</subtitle>
<id>https://git.shady.money/git/atom?h=jch</id>
<link rel='self' href='https://git.shady.money/git/atom?h=jch'/>
<link rel='alternate' type='text/html' href='https://git.shady.money/git/'/>
<updated>2026-02-20T23:03:42Z</updated>
<entry>
<title>symlinks: use unsigned int for flags</title>
<updated>2026-02-20T23:03:42Z</updated>
<author>
<name>Tian Yuchen</name>
<email>a3205153416@gmail.com</email>
</author>
<published>2026-02-16T17:20:28Z</published>
<link rel='alternate' type='text/html' href='https://git.shady.money/git/commit/?id=96286f14b0dc1a1c9ba4f6842d99ce738cc766da'/>
<id>urn:sha1:96286f14b0dc1a1c9ba4f6842d99ce738cc766da</id>
<content type='text'>
The 'flags' and 'track_flags' fields in symlinks.c are used
strictly as a collection of bits (using bitwise operators including
&amp;, |, ~). Using a signed integer for bitmasks may lead to undefined
behavior with shift operations and logic errors if the MSB is touched.

Change these fields from 'int' to 'unsigned int' to match our usage
patterns.

Signed-off-by: Tian Yuchen &lt;a3205153416@gmail.com&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
</content>
</entry>
<entry>
<title>global: mark code units that generate warnings with `-Wsign-compare`</title>
<updated>2024-12-06T11:20:02Z</updated>
<author>
<name>Patrick Steinhardt</name>
<email>ps@pks.im</email>
</author>
<published>2024-12-06T10:27:19Z</published>
<link rel='alternate' type='text/html' href='https://git.shady.money/git/commit/?id=41f43b8243f42b9df2e98be8460646d4c0100ad3'/>
<id>urn:sha1:41f43b8243f42b9df2e98be8460646d4c0100ad3</id>
<content type='text'>
Mark code units that generate warnings with `-Wsign-compare`. This
allows for a structured approach to get rid of all such warnings over
time in a way that can be easily measured.

Signed-off-by: Patrick Steinhardt &lt;ps@pks.im&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
</content>
</entry>
<entry>
<title>symlinks.h: move declarations for symlinks.c functions from cache.h</title>
<updated>2023-04-24T19:47:31Z</updated>
<author>
<name>Elijah Newren</name>
<email>newren@gmail.com</email>
</author>
<published>2023-04-22T20:17:09Z</published>
<link rel='alternate' type='text/html' href='https://git.shady.money/git/commit/?id=cb2a51356d3019582128a818aea533ccd11f42c0'/>
<id>urn:sha1:cb2a51356d3019582128a818aea533ccd11f42c0</id>
<content type='text'>
Signed-off-by: Elijah Newren &lt;newren@gmail.com&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
</content>
</entry>
<entry>
<title>setup.h: move declarations for setup.c functions from cache.h</title>
<updated>2023-03-21T17:56:54Z</updated>
<author>
<name>Elijah Newren</name>
<email>newren@gmail.com</email>
</author>
<published>2023-03-21T06:26:05Z</published>
<link rel='alternate' type='text/html' href='https://git.shady.money/git/commit/?id=e38da487cc50ce4b5b48085eebcab8268c541579'/>
<id>urn:sha1:e38da487cc50ce4b5b48085eebcab8268c541579</id>
<content type='text'>
Signed-off-by: Elijah Newren &lt;newren@gmail.com&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
</content>
</entry>
<entry>
<title>treewide: be explicit about dependence on gettext.h</title>
<updated>2023-03-21T17:56:51Z</updated>
<author>
<name>Elijah Newren</name>
<email>newren@gmail.com</email>
</author>
<published>2023-03-21T06:25:54Z</published>
<link rel='alternate' type='text/html' href='https://git.shady.money/git/commit/?id=f394e093df10f1867d9bb2180b3789ee61124aed'/>
<id>urn:sha1:f394e093df10f1867d9bb2180b3789ee61124aed</id>
<content type='text'>
Dozens of files made use of gettext functions, without explicitly
including gettext.h.  This made it more difficult to find which files
could remove a dependence on cache.h.  Make C files explicitly include
gettext.h if they are using it.

However, while compat/fsmonitor/fsm-ipc-darwin.c should also gain an
include of gettext.h, it was left out to avoid conflicting with an
in-flight topic.

Signed-off-by: Elijah Newren &lt;newren@gmail.com&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
</content>
</entry>
<entry>
<title>symlinks: do not include startup_info-&gt;original_cwd in dir removal</title>
<updated>2021-12-09T21:33:13Z</updated>
<author>
<name>Elijah Newren</name>
<email>newren@gmail.com</email>
</author>
<published>2021-12-09T05:08:29Z</published>
<link rel='alternate' type='text/html' href='https://git.shady.money/git/commit/?id=00fcce285db3db48f85730a183421fdb488c14cc'/>
<id>urn:sha1:00fcce285db3db48f85730a183421fdb488c14cc</id>
<content type='text'>
symlinks has a pair of schedule_dir_for_removal() and
remove_scheduled_dirs() functions that ensure that directories made
empty by removing other files also themselves get removed.  However, we
want to exclude startup_info-&gt;original_cwd and leave it around.  This
avoids the user getting confused by subsequent git commands (and non-git
commands) that would otherwise report confusing messages about being
unable to read the current working directory.

Acked-by: Derrick Stolee &lt;stolee@gmail.com&gt;
Acked-by: Ævar Arnfjörð Bjarmason &lt;avarab@gmail.com&gt;
Signed-off-by: Elijah Newren &lt;newren@gmail.com&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
</content>
</entry>
<entry>
<title>checkout: don't follow symlinks when removing entries</title>
<updated>2021-03-18T19:58:10Z</updated>
<author>
<name>Matheus Tavares</name>
<email>matheus.bernardino@usp.br</email>
</author>
<published>2021-03-18T18:43:47Z</published>
<link rel='alternate' type='text/html' href='https://git.shady.money/git/commit/?id=fab78a0c3defddff87ea5aa7dd32c5e444c43f1f'/>
<id>urn:sha1:fab78a0c3defddff87ea5aa7dd32c5e444c43f1f</id>
<content type='text'>
At 1d718a5108 ("do not overwrite untracked symlinks", 2011-02-20),
symlink.c:check_leading_path() started returning different codes for
FL_ENOENT and FL_SYMLINK. But one of its callers, unlink_entry(), was
not adjusted for this change, so it started to follow symlinks on the
leading path of to-be-removed entries. Fix that and add a regression
test.

Note that since 1d718a5108 check_leading_path() no longer differentiates
the case where it found a symlink in the path's leading components from
the cases where it found a regular file or failed to lstat() the
component. So, a side effect of this current patch is that
unlink_entry() now returns early in all of these three cases. And
because we no longer try to unlink such paths, we also don't get the
warning from remove_or_warn().

For the regular file and symlink cases, it's questionable whether the
warning was useful in the first place: unlink_entry() removes tracked
paths that should no longer be present in the state we are checking out
to. If the path had its leading dir replaced by another file, it means
that the basename already doesn't exist, so there is no need for a
warning. Sure, we are leaving a regular file or symlink behind at the
path's dirname, but this file is either untracked now (so again, no
need to warn), or it will be replaced by a tracked file during the next
phase of this checkout operation.

As for failing to lstat() one of the leading components, the basename
might still exist only we cannot unlink it (e.g. due to the lack of the
required permissions). Since the user expect it to be removed
(especially with checkout's --no-overlay option), add back the warning
in this more relevant case.

Signed-off-by: Matheus Tavares &lt;matheus.bernardino@usp.br&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
</content>
</entry>
<entry>
<title>symlinks: update comment on threaded_check_leading_path()</title>
<updated>2021-03-18T19:58:08Z</updated>
<author>
<name>Matheus Tavares</name>
<email>matheus.bernardino@usp.br</email>
</author>
<published>2021-03-18T18:43:46Z</published>
<link rel='alternate' type='text/html' href='https://git.shady.money/git/commit/?id=462b4e8dfd688b8964da77daf17b64da5bdc54ad'/>
<id>urn:sha1:462b4e8dfd688b8964da77daf17b64da5bdc54ad</id>
<content type='text'>
Since 1d718a5108 ("do not overwrite untracked symlinks", 2011-02-20),
the comment on top of threaded_check_leading_path() is outdated and no
longer reflects the behavior of this function. Let's updated it to avoid
confusions. While we are here, also remove some duplicated comments to
avoid similar maintenance problems.

Signed-off-by: Matheus Tavares &lt;matheus.bernardino@usp.br&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
</content>
</entry>
<entry>
<title>Sync with 2.20.5</title>
<updated>2021-02-12T14:49:35Z</updated>
<author>
<name>Johannes Schindelin</name>
<email>johannes.schindelin@gmx.de</email>
</author>
<published>2021-02-12T14:49:35Z</published>
<link rel='alternate' type='text/html' href='https://git.shady.money/git/commit/?id=b1726b1a38ee913cec7c72f4f4a5fe0c4b386386'/>
<id>urn:sha1:b1726b1a38ee913cec7c72f4f4a5fe0c4b386386</id>
<content type='text'>
* maint-2.20:
  Git 2.20.5
  Git 2.19.6
  Git 2.18.5
  Git 2.17.6
  unpack_trees(): start with a fresh lstat cache
  run-command: invalidate lstat cache after a command finished
  checkout: fix bug that makes checkout follow symlinks in leading path
</content>
</entry>
<entry>
<title>checkout: fix bug that makes checkout follow symlinks in leading path</title>
<updated>2021-02-12T14:47:02Z</updated>
<author>
<name>Matheus Tavares</name>
<email>matheus.bernardino@usp.br</email>
</author>
<published>2020-12-10T13:27:55Z</published>
<link rel='alternate' type='text/html' href='https://git.shady.money/git/commit/?id=684dd4c2b414bcf648505e74498a608f28de4592'/>
<id>urn:sha1:684dd4c2b414bcf648505e74498a608f28de4592</id>
<content type='text'>
Before checking out a file, we have to confirm that all of its leading
components are real existing directories. And to reduce the number of
lstat() calls in this process, we cache the last leading path known to
contain only directories. However, when a path collision occurs (e.g.
when checking out case-sensitive files in case-insensitive file
systems), a cached path might have its file type changed on disk,
leaving the cache on an invalid state. Normally, this doesn't bring
any bad consequences as we usually check out files in index order, and
therefore, by the time the cached path becomes outdated, we no longer
need it anyway (because all files in that directory would have already
been written).

But, there are some users of the checkout machinery that do not always
follow the index order. In particular: checkout-index writes the paths
in the same order that they appear on the CLI (or stdin); and the
delayed checkout feature -- used when a long-running filter process
replies with "status=delayed" -- postpones the checkout of some entries,
thus modifying the checkout order.

When we have to check out an out-of-order entry and the lstat() cache is
invalid (due to a previous path collision), checkout_entry() may end up
using the invalid data and thrusting that the leading components are
real directories when, in reality, they are not. In the best case
scenario, where the directory was replaced by a regular file, the user
will get an error: "fatal: unable to create file 'foo/bar': Not a
directory". But if the directory was replaced by a symlink, checkout
could actually end up following the symlink and writing the file at a
wrong place, even outside the repository. Since delayed checkout is
affected by this bug, it could be used by an attacker to write
arbitrary files during the clone of a maliciously crafted repository.

Some candidate solutions considered were to disable the lstat() cache
during unordered checkouts or sort the entries before passing them to
the checkout machinery. But both ideas include some performance penalty
and they don't future-proof the code against new unordered use cases.

Instead, we now manually reset the lstat cache whenever we successfully
remove a directory. Note: We are not even checking whether the directory
was the same as the lstat cache points to because we might face a
scenario where the paths refer to the same location but differ due to
case folding, precomposed UTF-8 issues, or the presence of `..`
components in the path. Two regression tests, with case-collisions and
utf8-collisions, are also added for both checkout-index and delayed
checkout.

Note: to make the previously mentioned clone attack unfeasible, it would
be sufficient to reset the lstat cache only after the remove_subtree()
call inside checkout_entry(). This is the place where we would remove a
directory whose path collides with the path of another entry that we are
currently trying to check out (possibly a symlink). However, in the
interest of a thorough fix that does not leave Git open to
similar-but-not-identical attack vectors, we decided to intercept
all `rmdir()` calls in one fell swoop.

This addresses CVE-2021-21300.

Co-authored-by: Johannes Schindelin &lt;johannes.schindelin@gmx.de&gt;
Signed-off-by: Matheus Tavares &lt;matheus.bernardino@usp.br&gt;
</content>
</entry>
</feed>
