I found two issues relating to extracting tar files where a file has
the setuid bit set:
- unsafe default
- setuid but lost
To demonstrate these issues, I created this simple C file:
--- cut here for setuid-demo.c ---
#include <stdio.h>
#include <unistd.h>
int main()
{
printf("running as id %d\n", (int)geteuid());
return 0;
}
--- cut here for setuid-demo.c ---
I compiled it, set the setuid bit, created a tar archive, and
inspected it:
$ gcc -Wall setuid-demo.c
$ chmod 4711 a.out
$ tar cf a.out.tar a.out
$ tar tvf a.out.tar
-rws--x--x cederp/cederp 8352 2019-02-04 11:42 a.out
Let's extract it using tar to see how it works:
$ rm a.out
$ tar xf a.out.tar
$ ls -l a.out
-rwx--x--x 1 cederp cederp 8352 feb 4 11:42 a.out*
Ok. We lost the setuid bit. But when we run as root we get it:
$ rm a.out
$ sudo tar xf a.out.tar
$ ls -l a.out
-rws--x--x 1 cederp cederp 8352 feb 4 11:42 a.out*
Now, let's do the same with Pike:
$ rm a.out
$ ~/rgit/pike/bin/pike
Pike v8.1 release 13 running Hilfe v3.5 (Incremental Pike Frontend)
> Filesystem.Tar("a.out.tar")->tar->extract("/", ".");
$ ls -l a.out
-rws--x--x 1 cederp cederp 8352 feb 4 11:42 a.out*
Looking good. But this is of course a flawed test. Let's see what
happens when we run this as root?
$ rm a.out
$ sudo ~/rgit/pike/bin/pike
Pike v8.1 release 13 running Hilfe v3.5 (Incremental Pike Frontend)
> Filesystem.Tar("a.out.tar")->tar->extract("/", ".");
$ ls -l a.out
-rws--x--x 1 root cederp 8352 feb 4 11:42 a.out*
Oh! I got a setuid-root file! This is the first problem: I don't
like that Filesystem.Tar by default extracts the setuid, setgid and
sticky bits, while it doesn't extract the user and group info.
Changing this would be backwards incompatible, though, so perhaps the
best thing you could do is document this.
But the issue is arguably in my code . I should have specified
EXTRACT_CHOWN if I wanted a root-extracted file to be owned by cederp.
Lets do that and see what happens:
$ rm a.out
$ sudo ~/rgit/pike/bin/pike
Pike v8.1 release 13 running Hilfe v3.5 (Incremental Pike Frontend)
> Filesystem.Tar("a.out.tar")->tar->extract(
"/", ".", 0, Filesystem.Tar.EXTRACT_CHOWN);
$ ls -l a.out
-rwx--x--x 1 cederp cederp 8352 feb 4 11:42 a.out*
The setuid bit is lost! This is because Filesystem.Tar first sets the
mode bits, and then sets the owner. Quoting chown(2):
When the owner or group of an executable file is changed
by an unprivileged user, the S_ISUID and S_ISGID mode
bits are cleared. POSIX does not specify whether this
also should happen when root does the chown(); the Linux
behavior depends on the kernel version, and since Linux
2.2.13, root is treated like other users.
It would work better to set the correct owner first, and then set the
mode bits.
