guix-patches
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[bug#50960] [PATCH 04/10] DRAFT shell: By default load the local 'guix.s


From: Ludovic Courtès
Subject: [bug#50960] [PATCH 04/10] DRAFT shell: By default load the local 'guix.scm' or 'manifest.scm' file.
Date: Mon, 04 Oct 2021 10:07:48 +0200
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/27.2 (gnu/linux)

Hi Maxime,

Maxime Devos <maximedevos@telenet.be> skribis:

> Ludovic Courtès schreef op za 02-10-2021 om 12:22 [+0200]:
>> +(define (find-file-in-parent-directories candidates)
>> +  "Find one of CANDIDATES in the current directory or one of its ancestors."
>> +  (let loop ((directory (getcwd)))
>> +    (and (= (stat:uid (stat directory)) (getuid))
>> +         (or (any (lambda (candidate)
>> +                    (let ((candidate (string-append directory "/" 
>> candidate)))
>> +                      (and (file-exists? candidate) candidate)))
>> +                  candidates)
>> +             (loop (string-append directory "/..")))))) ;Unix ".." 
>> resolution
>
> I do not recommend this.  What would happen if someone creates a temporary 
> directory
> "/tmp/stuff" do things in to throw away later (setting permissions 
> appropriately),
> tries to create a guix.scm in that directory but misspells it as, say, 
> guix.sm, and runs
> "guix shell" from within /tmp/stuff?  Then find-file-in-parent-directories 
> would
> load /tmp/guix.scm (possibly created by a local attacker, assuming a 
> multi-user system),
> -- if it weren't for the (= (stat:uid (stat directory)) (getuid)).
>
> Because of the (= (stat:uid ...) (getuid)), this attack method is not 
> possible.

Right.  :-)

In libgit2, ‘find_repo’ (called by ‘git_repository_discover’) stops at
device boundaries, which is wise.  But it doesn’t stop when the parent
has a different owner (!).

Unlike the code above, it does lexical “..” resolution after first
calling realpath(3) on the directory name; not sure what to think about
this.  (The code of Git itself is harder to read for me.)

> However, it causes other issues.  Now it isn't possible for two users (that 
> trust
> each other), to set up a directory writable by both (e.g. with ACLs, or by 
> making
> the directory group-writable and placing the two users in the same group), for
> working together, with a guix.scm usable by both.
>
> These can be two users on the same machine, or remotely via something like 
> NFS,
> or a single person having multiple user accounts used for different purposes.

Well, sure, but that’s a very uncommon scenario, isn’t it?

I was actually hesitant about this find-in-parent behavior.  I find it
convenient that ‘git’ does that, for instance, so I thought it might be
nice as well.

Thoughts?

Ludo’.





reply via email to

[Prev in Thread] Current Thread [Next in Thread]