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.