Checks for leaked file descriptors in do_exec in dbus/dbus-spawn.c and _dbus_close_all in dbus/dbus-sysdeps-unix.c miss files opened to a previously higher resource limit
Submitted by Steven Stewart-Gallus
Assigned to D-Bus Maintainers
Description
I noticed this issue while I was stracing my processes and it started to bug me. Checks for leaked file descriptors in do_exec in dbus/dbus-spawn.c and _dbus_close_all in dbus/dbus-sysdeps-unix.c miss files opened to a previously higher resource limit. Some code that illustrates having a file open that is higher than the resource limit reported by sysconf is below:
#include <errno.h> #include <stdlib.h> #include <stdio.h> #include <string.h> #include <sys/time.h> #include <sys/resource.h>
int main() { struct rlimit const limit = { .rlim_cur = 0, .rlim_max = 0 }; if (-1 == setrlimit(RLIMIT_NOFILE, &limit)) { fprintf(stderr, "error: %s\n", strerror(errno)); exit(EXIT_FAILURE); }
puts("Printing to standard output even though the resource limit is lowered past standard output's number!");
return EXIT_SUCCESS;
}
Luckily, on Linux _dbus_close_all tries to optimize using /proc/self/fd which catches this exception. Unfortunately, the code falls back to using the shitty and incorrect method of testing all files below the sysconf resource limit when /proc/self/fd is not available which may potentially be a security issue. Also, this doesn't work if sysconf reports the file limit as unlimited. Stupidly, the code just picks the random default of 1024 in the case where sysconf reports an unlimited file limit.
The code in do_exec which reports leaked file descriptors does not use /proc/self/fd at all and misses this corner case.
Unfortunately, a portable, correct and secure solution might need to use platform specific tricks (such as /proc/self/fd) in every case and never fallback to a compromise such as checking all files below a resource limit.
As as side note, the /proc/self/fd method breaks use of Valgrind (see issue https://bugs.kde.org/show_bug.cgi?id=331311).
As a side note, it may be smarter to move the file descriptor closing inside of the various DBus binaries instead of into the library code that spawns processes from them. That way, people who use those binaries directly will get protection for free.
Version: 1.5