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

[Enrico Scholz <enrico.scholz@sigma-chemnitz.de>]

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.


Enrico