@matheusbsbs / matheus-bagliano / commits / 2ef579e261

attr: do not respect symlinks for in-tree .gitattributes

The attributes system may sometimes read in-tree files from the filesystem, and sometimes from the index. In the latter case, we do not resolve symbolic links (and are not likely to ever start doing so). Let's open filesystem links with O_NOFOLLOW so that the two cases behave consistently. As a bonus, this means that git will not follow such symlinks to read and parse out-of-tree paths. In some cases this could have security implications, as a malicious repository can cause Git to open and read arbitrary files. It could already feed arbitrary content to the parser, but in certain setups it might be able to exfiltrate data from those paths (e.g., if an automated service operating on the malicious repo reveals its stderr to an attacker). Note that O_NOFOLLOW only prevents following links for the path itself, not intermediate directories in the path. At first glance, it seems like ln -s /some/path in-repo might still look at "in-repo/.gitattributes", following the symlink to "/some/path/.gitattributes". However, if "in-repo" is a symbolic link, then we know that it has no git paths below it, and will never look at its .gitattributes file. We will continue to support out-of-tree symbolic links (e.g., in $GIT_DIR/info/attributes); this just affects in-tree links. When a symbolic link is encountered, the contents are ignored and a warning is printed. POSIX specifies ELOOP in this case, so the user would generally see something like: warning: unable to access '.gitattributes': Too many levels of symbolic links Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Feb 16, 2021 at 09:44 UTC 2ef579e261fcd85a4eb6e0ce98ee4a01625fc210

2 files changed +48 -7

attr.c

+15 -4

@@ -280,6 +280,7 @@ static const char blank[] = " \t\r\n";

280

281 /* Flags usable in read_attr() and parse_attr_line() family of functions. */

282 #define READ_ATTR_MACRO_OK (1<<0)

283 +#define READ_ATTR_NOFOLLOW (1<<1)

284

285 /*

286 * Parse a whitespace-delimited attribute state (i.e., "attr",

@@ -704,13 +705,23 @@ void git_attr_set_direction(enum git_attr_direction new_direction)

705

706 static struct attr_stack *read_attr_from_file(const char *path, unsigned flags)

707 {

707 - FILE *fp = fopen_or_warn(path, "r");

708 + int fd;

709 + FILE *fp;

710 struct attr_stack *res;

711 char buf[2048];

712 int lineno = 0;

713

712 - if (!fp)

714 + if (flags & READ_ATTR_NOFOLLOW)

715 + fd = open_nofollow(path, O_RDONLY);

716 + else

717 + fd = open(path, O_RDONLY);

718 +

719 + if (fd < 0) {

720 + warn_on_fopen_errors(path);

721 return NULL;

722 + }

723 + fp = xfdopen(fd, "r");

724 +

725 res = xcalloc(1, sizeof(*res));

726 while (fgets(buf, sizeof(buf), fp)) {

727 char *bufp = buf;

@@ -870,7 +881,7 @@ static void bootstrap_attr_stack(const struct index_state *istate,

881 }

882

883 /* root directory */

873 - e = read_attr(istate, GITATTRIBUTES_FILE, flags);

884 + e = read_attr(istate, GITATTRIBUTES_FILE, flags | READ_ATTR_NOFOLLOW);

885 push_stack(stack, e, xstrdup(""), 0);

886

887 /* info frame */

@@ -961,7 +972,7 @@ static void prepare_attr_stack(const struct index_state *istate,

972 strbuf_add(&pathbuf, path + pathbuf.len, (len - pathbuf.len));

973 strbuf_addf(&pathbuf, "/%s", GITATTRIBUTES_FILE);

974

964 - next = read_attr(istate, pathbuf.buf, 0);

975 + next = read_attr(istate, pathbuf.buf, READ_ATTR_NOFOLLOW);

976

977 /* reset the pathbuf to not include "/.gitattributes" */

978 strbuf_setlen(&pathbuf, len);

t/t0003-attributes.sh

+33 -3

@@ -4,12 +4,16 @@ test_description=gitattributes

5 . ./test-lib.sh

7 -attr_check () {

7 +attr_check_basic () {

8 path="$1" expect="$2" git_opts="$3" &&

10 git $git_opts check-attr test -- "$path" >actual 2>err &&

11 echo "$path: test: $expect" >expect &&

12 - test_cmp expect actual &&

12 + test_cmp expect actual

13 +}

14 +

15 +attr_check () {

16 + attr_check_basic "$@" &&

17 test_must_be_empty err

18 }

@@ -331,7 +335,6 @@ test_expect_success 'binary macro expanded by -a' '

335 test_cmp expect actual

336 '

337

334 -

338 test_expect_success 'query binary macro directly' '

339 echo "file binary" >.gitattributes &&

340 echo file: binary: set >expect &&

@@ -339,4 +342,31 @@ test_expect_success 'query binary macro directly' '

342 test_cmp expect actual

343 '

344

345 +test_expect_success SYMLINKS 'set up symlink tests' '

346 + echo "* test" >attr &&

347 + rm -f .gitattributes

348 +'

349 +

350 +test_expect_success SYMLINKS 'symlinks respected in core.attributesFile' '

351 + test_when_finished "rm symlink" &&

352 + ln -s attr symlink &&

353 + test_config core.attributesFile "$(pwd)/symlink" &&

354 + attr_check file set

355 +'

356 +

357 +test_expect_success SYMLINKS 'symlinks respected in info/attributes' '

358 + test_when_finished "rm .git/info/attributes" &&

359 + ln -s ../../attr .git/info/attributes &&

360 + attr_check file set

361 +'

362 +

363 +test_expect_success SYMLINKS 'symlinks not respected in-tree' '

364 + test_when_finished "rm -rf .gitattributes subdir" &&

365 + ln -s attr .gitattributes &&

366 + mkdir subdir &&

367 + ln -s ../attr subdir/.gitattributes &&

368 + attr_check_basic subdir/file unspecified &&

369 + test_i18ngrep "unable to access.*gitattributes" err

370 +'

371 +

372 test_done