srg-imperial / SaBRe

Load-time selective binary rewriting
Other
152 stars 16 forks source link

Handle shebangs in scripts #80

Closed RiscInside closed 2 years ago

RiscInside commented 2 years ago

SaBRe will now detect if the client it runs is in fact a script. In this case, SaBRe will parse the shebang line, obtain the name of the interpreter (+ an optional argument), and restart SaBRe with modified args using execve. The limit on the shebang length is obtained from <linux/binfmts.h>.

$ cat ./test
#!/usr/bin/echo arg
$ ./sabre plugins/sbr-trace/libsbr-trace.so -- ./test
getrandom(0X07F315D25BFD8, 0X08, 0X01) = 0X08
brk(0X0) = 0X055BE10745000
brk(0X055BE10766000) = 0X055BE10766000
openat("/usr/lib/locale/locale-archive", O_RDONLY|O_CLOEXEC) = 0X03
newfstatat(0X03, 0X07F315D216F13, 0X07F315D256680, 0X01000) = 0X0
mmap(0X0, 0X02E85E0, PROT_READ, MAP_PRIVATE, 3, 0X0) = 0X07F315B0BF000
close(0X03) = 0X0
newfstatat(0X01, 0X07F315D216F13, 0X07FFDA1038530, 0X01000) = 0X0
arg ./test
write(1, "arg ./test\n", 0X0B) = 0X0B
close(0X01) = 0X0
close(0X02) = 0X0

handle_shebang does not try to check if the interpreter is in fact an ELF file. Otherwise, handle_shebang tries to match the handling of shebangs on Linux.

andronat commented 2 years ago

Is there a reason you prefer to forward the interpreter path to execve instead of directly calling the interpreter elf binary? The reason I'm asking because execve() -> Interpreter scripts is doing exactly this.

I'm slightly reluctant with execve-ing inside sabre because it is just another layer of complexity for debugging with gdb.

RiscInside commented 2 years ago

Initially, I thought that shebangs can pass arbitrarily many command-line arguments, so I decided to call into execve to avoid moving the entire stack frame. Now I know that there can be at most two additional arguments introduced by the shebang line (interpreter path + optional argument), so I can change it to rewriting the frame instead

andronat commented 2 years ago

I see. That's a good point. Actually then it is indeed better to call execve! I'll add some small comments in your code soon and then we can merge 🚀.

RiscInside commented 2 years ago

I changed it to not call execve since there is always enough room in the argv array for two more arguments

andronat commented 2 years ago

Thanks for your contribution @RiscInside! I'll slightly refactor the code and add a couple of LIT tests.