[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[bug#36699] [PATCH 4/4] channels: Reject directories with '..' in '.guix
[bug#36699] [PATCH 4/4] channels: Reject directories with '..' in '.guix-channel' file.
Thu, 18 Jul 2019 15:44:36 +0200
Gnus/5.13 (Gnus v5.13) Emacs/26.2 (gnu/linux)
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. :-)
bug#36699: [PATCH 0/4] Strengthen '.guix-channel' file handling, Ludovic Courtès, 2019/07/19