Re: [PATCH] patch.bbclass: Use one TMPDIR per patching process

[Richard Purdie <richard.purdie@linuxfoundation.org>]

On Fri, 2012-09-14 at 14:24 +0200, Enrico Scholz wrote:
> Richard Purdie <richard.purdie@linuxfoundation.org> writes:
> 
> >> > +    process_tmpdir = os.path.join('/tmp', str(os.getpid()))
> >> > +        shutil.rmtree(process_tmpdir)
> 
> > Its only being used as a prefix, not as the full directory path name
> > so it isn't quite as insecure as it would first appear.
> 
> It *is* insecure as it would first appear.  'shutil.rmtree()' does not
> traverse the directory in a secure way so that an attacker could:
> 
> 1. touch /tmp/<2-32767>/a
> 2. mkdir /tmp/<2-32767>/Z
> 3. wait for an inotify which triggers on deletion of the 'a' files
> 4. rmdir /tmp/$dir/Z
> 5. ln -s /home/<user> /tmp/$dir/Z
> 
> When steps 4+5 are executed between
> 
> | $ strace python -c 'import shutil; shutil.rmtree("/tmp/2");'
> | ...
> | lstat("/tmp/2/Z", {st_mode=S_IFDIR|0755, st_size=60, ...}) = 0
> | <<<< steps 4+5 here >>>>
> | openat(AT_FDCWD, "/tmp/2/Z", O_RDONLY|O_NONBLOCK|O_DIRECTORY|O_CLOEXEC) = 3
> | getdents(3, /* 3 entries */, 32768)     = 72
> | ...
> | unlink("/tmp/2/Z/foo")                  = 0
> 
> user writable directories will be removed.
> 
> There have been established some rules regarding secure tmpfile/dir
> generation in the last 10-20 years which should never be violated.
> 
> 
> Beside the obvious security issues, build will be aborted when somebody
> else creates a /tmp/<number> file and <number> matches the bitbake pid.

Firstly, I agree that we need to fix this and I know Constantin is
working on a patch. 

I would point out that the build process is likely full of such races
though. We execute an absolute *ton* of code, much of which is part of
upstream projects and which we don't directly control (think of all the
configure scripts and makefiles). I'd therefore suggest that builds be
considered insecure in themselves and run in environments appropriate to
that.

So build time security, I make *no* claims to and I find it hard to get
worked up about this lest it create some illusion builds are "secure".
Runtime security of the build output, very different question,
naturally.

Cheers,

Richard