On Sun, Nov 6, 2011 at 10:28 AM, Stefan Sperling <stsp_at_elego.de> wrote:
> On Sun, Nov 06, 2011 at 08:56:34AM -0500, Nico Kadel-Garcia wrote:
>> Stefan, if I may suggest, special-casing file names is leaving trouble
>> lying in wait.
>
> Doing so was apparently git's idea, not ours.
Writing tools to translate between API's as different as Subversion
and git is always going to be an adventure.
>> It would be safer to handle it via special file types.
>> /dev/null is a "character special" filetype in UNIX and Linux,
>> Character devices, block devices, and pipes should be ignored or
>> cause errors when source control tries to manipulate them.
>>
>> /dev/null is unusual in that it has write and read access to all
>> users, but I've seen too many cases where Subversion is being used by
>> root logins to administer system files and has privileges to
>> manipulate block devices, such as your disk drive. Rather than trying
>> to guess what names such devices should have, why not block operations
>> on a more powerful category that Subversion very clearly should not be
>> touching?
>
> Your concerns don't apply.
I realized some of this after I sent the note.
> Firstly, 'svn patch' won't touch anything outside of the working copy
> it is operating on (and yes, symlink attacks have been accounted for).
> 'svn patch' is not going to touch stuff in /dev or elsewhere even
> if you're root.
Not my point. Root users are sometimes idiots, and set up local clock
devices, and pipes, in some odd places. See below example.
> In the provided example, /dev/null was stripped to "dev/null".
> This resulted in a directory 'dev' and a file 'null' to be created in
> the working copy. Neither existed before the patch was applied.
> That's not wrong, it's basically what the patch says 'svn patch' should
> do. Expect that "/dev/null" has special meaning in git patches and
> therefore shouldn't be used as a patch target name.
Yup. The concern I was raising is for people who do clever things such
as use Subversion in chroot cage containing block devices, such as the
"/var/named/chroot" environment common to Bind setups. In fact, I've
done this to manage etc/named.conf and var/named/[whatever].zone files
in the same Subversion repository. I was cautious enough to use
'svn:ignore' to exclude "proc" and "dev" and "run' in order to avoid
block confusion about characther special devices, and pipes that are
contained in that chroot cage.
> Secondly, because Subversion's working copy meta data can only represent
> files, directories, and symlinks, this is all you'll ever get with
> 'svn patch'. A device node or a pipe inside the working copy will look
> like a regular unversioned node. And 'svn patch' won't touch unversioned
> nodes. 'svn add' refuses to add device nodes or pipes -- it errors out
> with "Unsupported node kind for path 'foo'."
*Good* that's useful. I've not tried this stunt, and was somewhat concerned.
> So I don't follow how the scenario you describe can even happen.
See above. If the The patch wouldn't be generated from the sytem's
working copy, but some fools's development working copy. In the theory
I raised, the generated the file or patch might attempt to overwrite
or be applied to an existing device, especially if being "added" by
the patch or if the person running Subversion accepted a "theirs-full"
patch. I'd hope that would generate an error or a patch. If device
nodes and pipes are protected by being "unversioned" and impossible to
"version", great! You folks were ahead of my concern, probably by
years.
/proc, though...... that one might more awkward to protect. Those are
nominally "files". Has anyone reviewed trying to protect /proc, or in
the case I mentioned /var/named/chroot/proc, from misapplied patches?
Received on 2011-11-07 00:49:39 CET