Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

4.18.16_2 kernel (default) incorrect setuid behaviour on x86_64-musl arch #4417

Closed
ghost opened this issue Nov 4, 2018 · 11 comments
Closed

Comments

@ghost
Copy link

ghost commented Nov 4, 2018

System

  • xuname: Void 4.18.16_2 x86_64-musl AuthenticAMD uptodate rFF
  • package: linux, linux4.18 linux4.18-4.18.16_2

Expected behavior

After a setuid(1000) call dropping root privileges, the program is no longer able to regain root privileges by setuid(0).

Actual behavior

After a setuid(1000) call dropping root privileges, the program is still able to regain root privileges by setuid(0). On the glibc architecture this error is not present: the second setuid(0) correctly fails.

Steps to reproduce the behavior

#define _GNU_SOURCE
#include <unistd.h>
#include <stdio.h>

int main(void) {
    uid_t r, e, s;

    getresuid(&r, &e, &s);
    printf("%d %d %d\n", r, e, s);

    if (setuid(1000) != 0)
        puts("setuid(1000) failed");
    else
        puts("setuid(1000) succeded");

    getresuid(&r, &e, &s);
    printf("%d %d %d\n", r, e, s);

    if (setuid(0) != 0)
        puts("setuid(0) failed");
    else
        puts("setuid(0) succeded");

    getresuid(&r, &e, &s);
    printf("%d %d %d\n", r, e, s);

    return 0;
}

compiled with gcc -o test test.c produces the output (when run as root)

0 0 0
setuid(1000) succeded
1000 1000 1000
setuid(0) succeded
0 0 0
@Hoshpak
Copy link
Member

Hoshpak commented Nov 4, 2018

Seems fixed with 4.19.0:

# uname -r
4.19.0_1
# ./test 
0 0 0
setuid(1000) succeded
1000 1000 1000
setuid(0) failed
1000 1000 1000

Would be interesting how it works out with 4.18.17 but that version hasn't hit the repos yet.

@Hoshpak
Copy link
Member

Hoshpak commented Nov 4, 2018

can't reproduce:

# uname -r
4.18.16_2
# ./test 
0 0 0
setuid(1000) succeded
1000 1000 1000
setuid(0) failed
1000 1000 1000

maybe it's a musl issue then. same result on x86_64-musl with either 4.18.16_2 or 4.19.0_1.

@ghost
Copy link
Author

ghost commented Nov 5, 2018

I have installed a fresh musl system on a separate disk, and the issue indeed does not reproduce there.
However, the issue on my own computer does still persist, and (after further testing) seems to be independent of the kernel version (persists on 4.14, 4.18 and 4.19).

What factors could cause this problem, other than the kernel, which I can test for?
This is the package list I have installed although I'm not sure how any other package could be affecting the kernel.

@pullmoll
Copy link
Member

pullmoll commented Nov 5, 2018

FWIW I can confirm it works as expected both on hardware and in a qemu Void installation.
Perhaps something is intercepting your syscalls? You could perhaps try to strace the test.

@ghost
Copy link
Author

ghost commented Nov 5, 2018

There doesn't seem to be anything suspicious going on - just the kernel allowing the second setuid (returns 0). This is the strace:

execve("./test", ["./test"], 0x7ffeabef0150 /* 20 vars */) = 0
arch_prctl(ARCH_SET_FS, 0x7f1838380b28) = 0
set_tid_address(0x7f1838380b68)         = 2105
mprotect(0x7f183837d000, 4096, PROT_READ) = 0
mprotect(0x56256ce02000, 4096, PROT_READ) = 0
getresuid([0], [0], [0])                = 0
ioctl(1, TIOCGWINSZ, {ws_row=78, ws_col=156, ws_xpixel=936, ws_ypixel=1014}) = 0
writev(1, [{iov_base="0 0 0", iov_len=5}, {iov_base="\n", iov_len=1}], 2) = 6
rt_sigprocmask(SIG_BLOCK, ~[RTMIN RT_1 RT_2], [], 8) = 0
rt_sigprocmask(SIG_BLOCK, ~[], NULL, 8) = 0
setuid(1000)                            = 0
futex(0x7f1838380fd8, FUTEX_WAKE_PRIVATE, 2147483647) = 0
rt_sigprocmask(SIG_SETMASK, [], NULL, 8) = 0
writev(1, [{iov_base="setuid(1000) succeded", iov_len=21}, {iov_base="\n", iov_len=1}], 2) = 22
getresuid([1000], [1000], [1000])       = 0
writev(1, [{iov_base="1000 1000 1000", iov_len=14}, {iov_base="\n", iov_len=1}], 2) = 15
rt_sigprocmask(SIG_BLOCK, ~[RTMIN RT_1 RT_2], [], 8) = 0
rt_sigprocmask(SIG_BLOCK, ~[], NULL, 8) = 0
setuid(0)                               = 0
futex(0x7f1838380fd8, FUTEX_WAKE_PRIVATE, 2147483647) = 0
rt_sigprocmask(SIG_SETMASK, [], NULL, 8) = 0
writev(1, [{iov_base="setuid(0) succeded", iov_len=18}, {iov_base="\n", iov_len=1}], 2) = 19
getresuid([0], [0], [0])                = 0
writev(1, [{iov_base="0 0 0", iov_len=5}, {iov_base="\n", iov_len=1}], 2) = 6
exit_group(0)                           = ?
+++ exited with 0 +++

I can also reproduce the setuid behaviour with this piece of asm

section .text

global _start

_start:
	mov rax, 105
	xor rdi, rdi
	syscall
	test eax, eax
	js exit

	mov rax, 105
	mov rdi, 1000
	syscall
	test eax, eax
	js exit

	mov rax, 105
	xor rdi, rdi
	syscall
exit:
	mov rdi, rax
	mov rax, 60
	syscall

which produces the following strace

execve("./testa", ["./testa"], 0x7ffd43c1e820 /* 17 vars */) = 0
setuid(0)                               = 0
setuid(1000)                            = 0
setuid(0)                               = 0
exit(0)                                 = ?
+++ exited with 0 +++

@pullmoll
Copy link
Member

pullmoll commented Nov 9, 2018

Here's the output of sudo strace -o log ./test; cat log in a VM running a x86_64-musl system with mate.

execve("./test", ["./test"], 0x7ffc5a87cdc0 /* 17 vars */) = 0
arch_prctl(ARCH_SET_FS, 0x7f632ffdcb28) = 0
set_tid_address(0x7f632ffdcb68)         = 1847
mprotect(0x7f632ffd9000, 4096, PROT_READ) = 0
mprotect(0x55d78689f000, 4096, PROT_READ) = 0
getresuid([0], [0], [0])                = 0
ioctl(1, TIOCGWINSZ, {ws_row=38, ws_col=212, ws_xpixel=0, ws_ypixel=0}) = 0
writev(1, [{iov_base="0 0 0", iov_len=5}, {iov_base="\n", iov_len=1}], 2) = 6
rt_sigprocmask(SIG_BLOCK, ~[RTMIN RT_1 RT_2], [], 8) = 0
rt_sigprocmask(SIG_BLOCK, ~[], NULL, 8) = 0
setuid(1000)                            = 0
futex(0x7f632ffdcfd8, FUTEX_WAKE_PRIVATE, 2147483647) = 0
rt_sigprocmask(SIG_SETMASK, [], NULL, 8) = 0
writev(1, [{iov_base="setuid(1000) succeded", iov_len=21}, {iov_base="\n", iov_len=1}], 2) = 22
getresuid([1000], [1000], [1000])       = 0
writev(1, [{iov_base="1000 1000 1000", iov_len=14}, {iov_base="\n", iov_len=1}], 2) = 15
rt_sigprocmask(SIG_BLOCK, ~[RTMIN RT_1 RT_2], [], 8) = 0
rt_sigprocmask(SIG_BLOCK, ~[], NULL, 8) = 0
setuid(0)                               = -1 EPERM (Operation not permitted)
futex(0x7f632ffdcfd8, FUTEX_WAKE_PRIVATE, 2147483647) = 0
rt_sigprocmask(SIG_SETMASK, [], NULL, 8) = 0
writev(1, [{iov_base="setuid(0) failed", iov_len=16}, {iov_base="\n", iov_len=1}], 2) = 17
getresuid([1000], [1000], [1000])       = 0
writev(1, [{iov_base="1000 1000 1000", iov_len=14}, {iov_base="\n", iov_len=1}], 2) = 15
exit_group(0)                           = ?
+++ exited with 0 +++

So setuid(0) fails with -EPERM as expected.
I really don't know how this could succeed in your setup.

@ghost
Copy link
Author

ghost commented Nov 10, 2018

@pullmoll that's exactly my point!
Since I've posted this bug report, I have reinstalled void in an effort to fix the issue, and although in a fresh install this isn't reproducible, somewhere along the way to getting my working system, the issue appeared, and it is still persistent now.
Unfortunately, I wasn't paying enough attention to pinpoint exactly the change which caused this functionality to break. I'll try to reproduce my installation on another disk and find out where this problem comes from.

@ghost
Copy link
Author

ghost commented Nov 10, 2018

I think I have found the culprit. Quite bizarrely, it seems to be loading the pam_rundir.so module.
If I load it with session optional pam_rundir.so in the /etc/pam.d/system-login file, the incorrect setuid behavior persists. If I comment out the line (by adding # in front), I can no longer reproduce the bug.

pkgver: pam_rundir-1.0.0_2
Should I therefore report this in a different issue (closing this one), or just continue on here, and possibly edit the first post?

@jnbr
Copy link
Contributor

jnbr commented Nov 10, 2018

Can you rebuild pam_rundir without the patch that ships with our template and test again?

jnbr added a commit to jnbr/void-packages that referenced this issue Nov 11, 2018
setting SECBIT_NO_SETUID_FIXUP in a pam module is a bad idea:
void-linux#4417
jnbr added a commit to jnbr/void-packages that referenced this issue Nov 11, 2018
setting SECBIT_NO_SETUID_FIXUP in a pam module is a bad idea:
void-linux#4417
jnbr added a commit that referenced this issue Nov 11, 2018
setting SECBIT_NO_SETUID_FIXUP in a pam module is a bad idea:
#4417
@jnbr
Copy link
Contributor

jnbr commented Nov 11, 2018

Thank you for reporting this. I removed the "fix" from pam_rundir which caused this.

@jnbr jnbr closed this as completed Nov 11, 2018
@ghost ghost mentioned this issue Nov 11, 2018
@ghost
Copy link
Author

ghost commented Nov 11, 2018

Removing the patch does indeed fix the issue, but now renders pam_rundir unusable (basically why this patch was introduced #408)

I have submitted a patch to fix the aforementioned issues in a different way jjk-jacky/pam_rundir#4
Should I also submit a pull request to this repo before the author merges changes to pam_rundir?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants