Gallopsled / pwntools

CTF framework and exploit development library
http://pwntools.com
Other
12.05k stars 1.71k forks source link

sendfile doesn't seems to be able to read /proc/self/maps in recent kernel version #1871

Closed Frefreak closed 2 years ago

Frefreak commented 3 years ago

Hi, after some experiment it seems sendfile system call can not read /proc/self/maps anymore in recent kernel version (tested in archlinux x86 5.11, archlinux arm 5.10). Here is a simple c code to test:

#include <stdlib.h>
#include <unistd.h>
#include <sys/sendfile.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <stdio.h>

int main(int argc, char *argv[])
{
    if (argc != 2) {
        printf("Usage: %s <file>\n", argv[0]);
        exit(1);
    }
    int fd = open(argv[1], O_RDONLY);
    int out_fd = creat("/dev/stdout", S_IRUSR|S_IWUSR);  
    if (fd < 0) {
        perror("open file");
        exit(1);
    }
    if (out_fd < 0) {
        perror("open stdin");
        exit(1);
    }
    int ret = sendfile(out_fd, fd, NULL, 1024);
    if (ret == -1) {
        perror("sendfile");
        exit(1);
    }

    return 0;
}

It prints "invalid argument" in 5.10/5.11 but works fine in a 4.15 system. I could not find the source of this change, so I'm not sure if this is some kind of upstream bug or is intended. However this renders shellcraft amd64 cat stops working, which has an influence on ELF libs/libc (alwasy empty or None).

Arusekk commented 3 years ago

Apparently, I just tested that amd64 has the exact same issue.

Frefreak commented 3 years ago

So maybe its related to this commit: https://github.com/torvalds/linux/commit/36e2c7421f02 According to some discussion in LKML mails, splice support for some of the proc files were added, but not including /proc/pid/maps. However even if this is supported, supplying stdout fd (1) as out_fd directly will also result in an invalid argument.

Arusekk commented 3 years ago

It may be related to stdout being a terminal. Does it work if you invoke the program with an explicit pipe or socket at stdout? Like ./a.out | cat or socat - exec:./a.out (don't use creat("/dev/stdout") for it, use dup(STDOUT_FILENO)).

Frefreak commented 3 years ago

Yep, that seems to be the issue. I tested in more systems and only one virtualbox vm needs to piping to cat or using socat to make it works. Others I can just invoking on the command line directly without issue. Not sure why though. So now the problem remains is that sendfile can't read /proc/self/maps...

Arusekk commented 3 years ago

This looks bad, since neither copy_file_range(2) nor splice(2) look like an option to us now, and the shellcode does not want to allocate super-much memory, especially if it resides on stack; we would need to agree on some buffer size, I think 4096 (single page) would be best, but should it be on stack? Or mmapped? Or should we mmap the input file instead of reading it?

Arusekk commented 3 years ago

I think the CAT_PROC_MAPS_EXIT pre-existing shellcodes can be replaced with mmap-then-write (or a read-write loop if they also broke seq_ops mmap, not tested), but I would leave shellcraft.linux.cat using sendfile and provide a separate slowcat/copycat/reliable_cat that would allocate a stack page (or mmap some anon memory) and do a read-write loop. Or maybe better cat, mmapcat and readcat doing the respective. It would be terrible to have read-write for CAT_PROC_MAPS_EXIT, though, since pwntools' ELF handling is pretty slow already and any low-hanging speedups are invaluable.

Note that I especially don't like the very fact this was broken in the kernel. I hope someone submits a patch adding it back (looks like some trivial modifications), maybe even I will if I manage to find some spare free time. So I think this is a kernel bug, and we probably can work it around, but should be treated as a kernel bug. Here is our punishment for not testing pwntools on kernel release candidates.

Is it exposed on some test in our test suite? Do they run fine in your containers?

Frefreak commented 3 years ago

I agree with you. It would be the best if they could add it back in kernel, and it seems reasonable given some other proc files' support are added back already. As for the tests I will take a run in my environment later and report the result.

Frefreak commented 3 years ago

I need to add a RUN sudo apt-get update in dockerfile in order to make the image successfully built. After running test in docker there're 15 tests failed, 14 of them are in elf/corefile which I believe is because of missing command coredumpctl. The last failed case is indeed revealing the problem in this issue (bash.libs is empty, and a manual repl run behaves the same):

File "../../pwnlib/elf/elf.py", line ?, in default
Failed example:
    any(map(lambda x: 'libc' in x, bash.libs.keys()))
Expected:
    True
Got:
    False
/usr/bin/x86_64-linux-gnu-objcopy: /tmp/pwn-asm-7vmw302o/sty1GluE: section .A lma 0x8049074 adjusted to 0x804a054
Arusekk commented 3 years ago

My first submitted kernel patch; hope they like it. 😅 You can watch the lkml thread if you want; probably few will notice the patch through the storm of @umn.edu reverts.

Frefreak commented 3 years ago

You can watch the lkml thread

will do :) This looks promising

heapcrash commented 3 years ago

Thanks @Arusekk for submitting the patch to the kernel! Hopefully it gets integrated quickly, else I suspect we'll see lots of issue requests for this.

It would be a LOT of work, but we can change the CAT_PROC_MAPS_EXIT shellcode to work correctly in all cases, since there are no shellcode size restrictions.

This would require us writing a special-case that does a stack reservation, read(2), and write(2), for each individual architecture, and maintaining it.

I believe that the current code is just a hard-coded copy of e.g. shellcraft.arm.linux.cat("/proc/self/maps") so there's no maintenance burden. Hard-coding was done for (1) speed [don't need to assemble at exploit-runtime] and (2) avoiding the need for the user to have the correct assembler to use it.

We could call the resulting shellcraft template catproc.asm. I would support backporting such a change to stable.

If we do, it might be worth adding a specific check for "/proc" to each cat.asm and have it emit a warning message that the generated shellcode may not work on kernel version XYZ, and suggest they use shellcraft.catproc instead.

Frefreak commented 3 years ago

It appears the proposal to add sendfile support back to /proc/pid/maps won't be accepted, at least in the near future :( So for now maybe the only thing we can do is to change the shellcode in order to deal with this?

heapcrash commented 3 years ago

I talked with @Arusekk a bit, and I think the approach we'll adopt will be to rewrite the CAT_PROC_PID_MAPS shellcode to use read and write using the stack as a buffer.

One way we discussed to achieve this is to modify cat.asm for each supported architecture to have a mode= operation, which uses sendfile by default but if mode='compatible' (or some other such flag) is specified, doing open / read / write will be used instead.

This requires writing new shell code for all of the architectures, and might take a while since it's a significant time investment.

heapcrash commented 3 years ago

Additional restrictions on LD_TRACE_LOADED_OBJECTS is that it doesn't work for BSD, musl linker, or Android crazy_linker.

A workaround is to use gdb.find_module_addresses, and load the ELFs yourself. However, this does not work for cross-arch-emulated-with-qemu binaries because it launches GDB directly rather than using gdb.debug() with a gdbscript. It also requires you have a GDB capable of debugging the binary (e.g. vanilla GDB might not support mips64).