linux-kernel - Re: [PATCH] apply: refuse touching a file beyond symlink

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <xmqq1tmcc9l9.fsf@gitster.dls.corp.google.com>
Date:	Fri, 30 Jan 2015 12:11:30 -0800
From:	Junio C Hamano <gitster@...ox.com>
To:	Jeff King <peff@...f.net>
Cc:	Git Mailing List <git@...r.kernel.org>,
	Josh Boyer <jwboyer@...oraproject.org>,
	"Linux-Kernel\@Vger. Kernel. Org" <linux-kernel@...r.kernel.org>,
	twaugh@...hat.com, Linus Torvalds <torvalds@...ux-foundation.org>
Subject: Re: [PATCH] apply: refuse touching a file beyond symlink

Junio C Hamano <gitster@...ox.com> writes:

> Jeff King <peff@...f.net> writes:
>
>>> +	if (!patch->is_delete && path_is_beyond_symlink(patch->new_name))
>>> +		return error(_("affected file '%s' is beyond a symbolic link"),
>>> +			     patch->new_name);
>>
>> Why does this not kick in when deleting a file? If it is not OK to
>> add across a symlink, why is it OK to delete?
>
> Hmph, adding
>
> 	if (patch->is_delete &&	path_is_beyond_symlink(patch->old_name))
> 		return error(_("deleted file '%s' is beyond a symlink"),
> 				patch->old_name);
>
> seems to break t4114.11, which wants to apply this patch to a tree
> that does not have a symbolic link but a directory at 'foo/'.
>
> diff --git a/foo b/foo
> new file mode 120000
> index 0000000..ba0e162
> --- /dev/null
> +++ b/foo
> @@ -0,0 +1 @@
> +bar
> \ No newline at end of file
> diff --git a/foo/baz b/foo/baz
> deleted file mode 100644
> index 682c76b..0000000
> --- a/foo/baz
> +++ /dev/null
> @@ -1 +0,0 @@
> -if only I knew

I am not sure how to fix this, without completely ripping out the
misguided "We should be able to concatenate outputs from multiple
invocations of 'git diff' into a single file and apply the result
with a single invocation of 'git apply'" change I grudgingly
accepted long time ago (7a07841c (git-apply: handle a patch that
touches the same path more than once better, 2008-06-27).

"git diff" output is designed each patch to apply independently to
the preimage to produce the postimage, and that allows patches to
two files can be swapped via -Oorderfile mechanism, and also "X was
created by copying from Y and Y is modified in place" will result in
X with the contents of Y in the preimage (i.e. before the in-place
modification of Y in the same patch) regardless of the order of X
and Y in the "git diff" output.  The above input used by t4114.11
expects to remove 'foo/baz' (leaving an empty directory foo as an
result but we do not track directories so it can be nuked to make
room if other patch in the same input wants to put something else,
either a regular file or a symbolic link, there) and create a blob
at 'foo', and such an input should apply regardless of the order of
patches in it.

The in_fn_table[] stuff broke that design completely.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/