Junio C Hamano <gitster@xxxxxxxxx> writes: > Michael Haggerty <mhagger@xxxxxxxxxxxx> writes: > >> When reading a symbolic ref via resolve_gitlink_ref_recursive(), check >> that the reference name that is pointed at is formatted correctly, >> using the same check as resolve_ref_unsafe() uses for non-gitlink >> references. This prevents bogosity like >> >> ref: ../../other/file >> >> from causing problems. > > I do agree that a textual symref "ref: ../../x/y" that is stored in > ".git/HEAD" or in ".git/refs/L" will step outside ".git/" and it is > problematic. But if ".git/refs/heads/a/b/LINK" has "ref: ../../x" > in it, shouldn't we interpret it as referring to the ref at > "refs/heads/x"? Actually, the textual symrefs have been invented to replace symbolic links used for .git/HEAD on symlink-incapable filesystems, and we do even not let the filesystem follow symlinks. The rule we have there are like so: /* Follow "normalized" - ie "refs/.." symlinks by hand */ if (S_ISLNK(st.st_mode)) { len = readlink(path, buffer, sizeof(buffer)-1); if (len < 0) { if (errno == ENOENT || errno == EINVAL) /* inconsistent with lstat; retry */ goto stat_ref; else return NULL; } buffer[len] = 0; if (starts_with(buffer, "refs/") && !check_refname_format(buffer, 0)) { strcpy(refname_buffer, buffer); refname = refname_buffer; if (flag) *flag |= REF_ISSYMREF; continue; } } So we should do exactly the same check, I would think, no? In a typical clone, the ".git/refs/remotes/origin/HEAD" textual symref stores "ref: refs/remotes/origin/master" and it is neither "ref: master" nor "ref: ./master", so it should be sensible to insist on "must start with 'refs/' and its format valid." -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html