Skip to content

Commit 339e060

Browse files
committed
Merge branch 'jk/symlink-out-of-tree' into HEAD
* jk/symlink-out-of-tree: optionally block out-of-repo symlinks
2 parents ff43bd9 + 685f86a commit 339e060

8 files changed

Lines changed: 195 additions & 3 deletions

File tree

apply.c

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4511,11 +4511,13 @@ static int try_create_file(struct apply_state *state, const char *path,
45114511
return !!mkdir(path, 0777);
45124512
}
45134513

4514-
if (has_symlinks && S_ISLNK(mode))
4514+
if (has_symlinks && S_ISLNK(mode)) {
45154515
/* Although buf:size is counted string, it also is NUL
45164516
* terminated.
45174517
*/
4518-
return !!symlink(buf, path);
4518+
prepare_repo_settings(state->repo);
4519+
return !!safe_symlink(state->repo, buf, path);
4520+
}
45194521

45204522
fd = open(path, O_CREAT | O_EXCL | O_WRONLY, (mode & 0100) ? 0777 : 0666);
45214523
if (fd < 0)

entry.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
#include "odb.h"
55
#include "odb/streaming.h"
66
#include "dir.h"
7+
#include "path.h"
78
#include "environment.h"
89
#include "gettext.h"
910
#include "hex.h"
@@ -324,7 +325,7 @@ static int write_entry(struct cache_entry *ce, char *path, struct conv_attrs *ca
324325
if (!has_symlinks || to_tempfile)
325326
goto write_file_entry;
326327

327-
ret = symlink(new_blob, path);
328+
ret = safe_symlink(the_repository, new_blob, path);
328329
free(new_blob);
329330
if (ret)
330331
return error_errno("unable to create symlink %s", path);

path.c

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1542,6 +1542,79 @@ int looks_like_command_line_option(const char *str)
15421542
return str && str[0] == '-';
15431543
}
15441544

1545+
static int symlink_leaves_repo(const char *target, const char *linkpath)
1546+
{
1547+
/*
1548+
* Absolute paths are always considered to leave the repository (even
1549+
* if they happen to point to the working tree path).
1550+
*/
1551+
if (is_absolute_path(target))
1552+
return 1;
1553+
1554+
/*
1555+
* Allow relative paths that start with a sequence of "../",
1556+
* as long as they do not break out of the symlink's root.
1557+
* This loop will detect break-out cases and return; otherwise, at the
1558+
* end of the loop "target" will point to the first non-".." component.
1559+
*
1560+
* We count the depth of linkpath by eating up directory components left
1561+
* to right. Technically the symlink would resolve right-to-left, but
1562+
* we don't care about the actual values, only the number.
1563+
*/
1564+
while (target[0] == '.') {
1565+
if (!target[1]) {
1566+
/* trailing "." -- ignore */
1567+
target++;
1568+
} else if (is_dir_sep(target[1])) {
1569+
/* "./" -- ignore */
1570+
target += 2;
1571+
} else if (target[1] == '.' &&
1572+
(!target[2] || is_dir_sep(target[2]))) {
1573+
/* ".." or "../" -- drop one from linkpath depth */
1574+
while (!is_dir_sep(*linkpath)) {
1575+
/* end-of-string; target exceeded our depth */
1576+
if (!*linkpath)
1577+
return 1;
1578+
linkpath++;
1579+
}
1580+
/* skip final "/" */
1581+
linkpath++;
1582+
1583+
/* skip past ".." */
1584+
target += 2;
1585+
/* and "/" if present */
1586+
if (is_dir_sep(*target))
1587+
target++;
1588+
}
1589+
}
1590+
1591+
/*
1592+
* Now we have a path in "target" that only go down into the tree.
1593+
* Disallow any interior "../", like "foo/../bar". These might be
1594+
* OK, but we cannot know unless we know whether "foo" is itself a
1595+
* symlink. So err on the side of caution.
1596+
*/
1597+
while (*target) {
1598+
const char *v;
1599+
if (skip_prefix(target, "..", &v) && (!*v || is_dir_sep(*v)))
1600+
return 1;
1601+
target++;
1602+
}
1603+
1604+
return 0;
1605+
}
1606+
1607+
int safe_symlink(struct repository *r, const char *target, const char *linkpath)
1608+
{
1609+
if (!r->settings.allow_external_symlinks &&
1610+
symlink_leaves_repo(target, linkpath)) {
1611+
errno = EPERM;
1612+
return -1;
1613+
}
1614+
1615+
return symlink(target, linkpath);
1616+
}
1617+
15451618
char *xdg_config_home_for(const char *subdir, const char *filename)
15461619
{
15471620
const char *home, *config_home;

path.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -262,6 +262,8 @@ enum scld_error safe_create_leading_directories_no_share(char *path);
262262
int safe_create_file_with_leading_directories(struct repository *repo,
263263
const char *path);
264264

265+
int safe_symlink(struct repository *, const char *target, const char *linkpath);
266+
265267
# ifdef USE_THE_REPOSITORY_VARIABLE
266268
# include "strbuf.h"
267269
# include "repository.h"

repo-settings.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,8 @@ void prepare_repo_settings(struct repository *r)
8484
&r->settings.pack_use_bitmap_boundary_traversal,
8585
r->settings.pack_use_bitmap_boundary_traversal);
8686
repo_cfg_bool(r, "core.usereplacerefs", &r->settings.read_replace_refs, 1);
87+
repo_cfg_bool(r, "core.allowexternalsymlinks",
88+
&r->settings.allow_external_symlinks, 1);
8789

8890
/*
8991
* The GIT_TEST_MULTI_PACK_INDEX variable is special in that

repo-settings.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,8 @@ struct repo_settings {
7070
int max_allowed_tree_depth;
7171

7272
char *hooks_path;
73+
74+
int allow_external_symlinks;
7375
};
7476
#define REPO_SETTINGS_INIT { \
7577
.shared_repository = -1, \

t/meson.build

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -275,6 +275,7 @@ integration_tests = [
275275
't2026-checkout-pathspec-file.sh',
276276
't2027-checkout-track.sh',
277277
't2030-unresolve-info.sh',
278+
't2031-checkout-symlink-external.sh',
278279
't2050-git-dir-relative.sh',
279280
't2060-switch.sh',
280281
't2070-restore.sh',
Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
#!/bin/sh
2+
3+
test_description='detection and prevention of out-of-tree symlinks'
4+
GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
5+
export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
6+
. ./test-lib.sh
7+
8+
if ! test_have_prereq SYMLINKS
9+
then
10+
skip_all='skipping external symlink tests (missing SYMLINKS)'
11+
test_done
12+
fi
13+
14+
create_symlink() {
15+
symlink=$1
16+
target=$2
17+
test_expect_success "create symlink ($symlink)" '
18+
sha1=$(printf "%s" "$target" | git hash-object -w --stdin) &&
19+
git update-index --add --cacheinfo "120000,$sha1,$symlink"
20+
'
21+
}
22+
23+
check_symlink () {
24+
symlink=$1
25+
config=$2
26+
outcome=$3
27+
expect=$4
28+
29+
if test "$outcome" = "allow"
30+
then
31+
fail=
32+
: ${expect:=test_cmp ../target}
33+
else
34+
fail=test_must_fail
35+
: ${expect:=! cat}
36+
fi
37+
38+
test_expect_success " check symlink ($symlink, $config -> $outcome)" "
39+
rm -f $symlink &&
40+
$fail git -c core.allowExternalSymlinks=$config \\
41+
checkout-index -- $symlink &&
42+
$expect $symlink
43+
"
44+
}
45+
46+
# we want to try breaking out of the repository,
47+
# so let's work inside a sub-repository, and break
48+
# out to the top-level trash directory
49+
test_expect_success 'set up repository' '
50+
echo content >target &&
51+
git init subrepo &&
52+
cd subrepo &&
53+
test_commit base &&
54+
echo content >in-repo-target
55+
'
56+
57+
create_symlink in-repo in-repo-target
58+
check_symlink in-repo false allow
59+
60+
create_symlink subdir/in-repo ../in-repo-target
61+
check_symlink subdir/in-repo false allow
62+
63+
create_symlink absolute "$TRASH_DIRECTORY/target"
64+
check_symlink absolute true allow
65+
check_symlink absolute false forbid
66+
67+
create_symlink relative "../target"
68+
check_symlink relative true allow
69+
check_symlink relative false forbid
70+
71+
create_symlink curdir .
72+
check_symlink curdir false allow test_path_is_dir
73+
create_symlink sneaky curdir/../target
74+
check_symlink sneaky true allow
75+
check_symlink sneaky false forbid
76+
77+
test_expect_success 'applying a patch checks symlink config' '
78+
git diff-index -p --cached HEAD -- relative >patch &&
79+
rm -f relative &&
80+
git -c core.allowExternalSymlinks=true apply <patch &&
81+
test_cmp ../target relative &&
82+
rm -f relative &&
83+
test_must_fail git -c core.allowExternalSymlinks=false apply <patch
84+
'
85+
86+
test_expect_success 'merge-recursive checks symlinks config' '
87+
git reset --hard &&
88+
89+
# create rename situation which requires processing
90+
# outside of unpack_trees()
91+
ln -s ../foo one &&
92+
git add one &&
93+
git commit -m base &&
94+
95+
ln -sf ../target one &&
96+
git commit -am modify &&
97+
98+
git checkout -b side HEAD^ &&
99+
git mv one two &&
100+
git commit -am rename &&
101+
102+
git -c core.allowExternalSymlinks=true merge main &&
103+
test_cmp ../target two &&
104+
105+
git reset --hard HEAD^ &&
106+
test_must_fail git -c core.allowExternalSymlinks=false merge main
107+
'
108+
109+
test_done

0 commit comments

Comments
 (0)