[bug#36699] [PATCH 4/4] channels: Reject directories with '..' in '.guix-channel' file.

[Ludovic Courtès]

Hi,

Danny Milosavljevic <address@hidden> skribis:

> On Wed, 17 Jul 2019 01:29:39 +0200
> Ludovic Courtès <address@hidden> wrote:
>
>> Ludovic Courtès <address@hidden> skribis:
>> 
>> > +  (define (sane-directory directory)
>> > +    ;; If DIRECTORY contains '..', raise an error; otherwise return it.
>> > +    (when (member ".." (string-split directory #\/))
>> > +      (raise (condition
>> > +              (&message (message "channel sub-directory must not contain 
>> > '..'"))
>> > +              (&error-location (location location)))))
>> > +    directory)  
>> 
>> On second thought, it’s probably kind of useless since the only place
>> where ‘directory’ is used is in the derivation that builds the channel,
>> which is normally running in a chroot:
>> 
>>   (let* ((subdir #$directory)
>>          (source (string-append #$source subdir)))
>>     (compile-files source go (find-files source "\\.scm$"))
>>     (mkdir-p (dirname scm))
>>     (symlink (string-append #$source subdir) scm))
>> 
>> So I guess we can drop this patch.  Thoughts?
>
> I generally don't like weird name matching like this.  The Linux VFS can do
> arbitrary things (which would complicate the situation) to the name tree.
> Even now, a symlink "x" to ".." would work and not be caught.  To say nothing
> of what a custom file system could do.
>
> Why single out this one way?  It gives the illusion of security.
>
> Containers are better indeed.

Yes, and since that’s what we have, we can forget about this patch.

I definitely agree with everything you wrote; it’s just that the kernel
Linux being what it is, one sometimes have to resort to hacks like this.
Fortunately, that was misguided here, so let’s forget about this.  :-)

Ludo’.