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

[Danny Milosavljevic]

Hi Ludo,

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.

Except when the match is not for security but only for usability, then I'm
fine with it (and then it should be a warning - who knows, maybe ".." means
"current directory" in WeirdFS :->).

pgpT1zlQD7WFR.pgp
Description: OpenPGP digital signature