Ensure STDIN/STDOUT/STDERR always exist
File descriptor confusion can result if a process is exec()d and
STDIN/STDOUT/STDERR do not exist. In those situations, the first,
second, and third files opened by the exec()d application will have FD
0, 1, and 2 respectively. Code which reads / writes to these STD* file
descriptors may end up reading / writing to unintended files.
To prevent this, guarantee that FDs 0, 1, and 2 always exist. Bionic
only currently guarantees this for AT_SECURE programs (eg, a setuid
binary, setgid binary, filesystem capabilities, or SELinux domain
transition).
Extending this to all exec()s adds robustness against this class of
bugs. Additionally, it allows a caller to do:
close(STDIN_FILENO);
close(STDOUT_FILENO);
close(STDERR_FILENO);
and know that the exec()d process will reopen these file descriptors on
its own. This has the potential to simplify other parts of Android, eg
https://android-review.googlesource.com/c/platform/system/apex/+/915694
Steps to reproduce:
sleep 100 <&- >&- 2>&- & BGPID=$! && ls -la /proc/$BGPID/fd && kill $BGPID
Expected:
$ sleep 100 <&- >&- 2>&- & BGPID=$! && ls -la /proc/$BGPID/fd && kill $BGPID
[1] 3154
total 0
dr-x------ 2 shell shell 0 1970-04-17 12:15 .
dr-xr-xr-x 9 shell shell 0 1970-04-17 12:15 ..
lrwx------ 1 shell shell 64 1970-04-17 12:15 0 -> /dev/null
lrwx------ 1 shell shell 64 1970-04-17 12:15 1 -> /dev/null
lrwx------ 1 shell shell 64 1970-04-17 12:15 2 -> /dev/null
$
[1] + Terminated \sleep 100 <&- >&- 2>&-
Actual:
$ sleep 100 <&- >&- 2>&- & BGPID=$! && ls -la /proc/$BGPID/fd && kill $BGPID
[1] 16345
total 0
dr-x------ 2 shell shell 0 2019-02-28 20:22 .
dr-xr-xr-x 9 shell shell 0 2019-02-28 20:22 ..
$
[1] + Terminated \sleep 100 <&- >&- 2>&-
Test: manual (see above)
Change-Id: I3e05700a1e8ebc7fc9d192211dd9fc030cc40139
diff --git a/libc/bionic/libc_init_common.cpp b/libc/bionic/libc_init_common.cpp
index f1fbfa9..b229cda 100644
--- a/libc/bionic/libc_init_common.cpp
+++ b/libc/bionic/libc_init_common.cpp
@@ -298,11 +298,26 @@
unsigned long is_AT_SECURE = getauxval(AT_SECURE);
if (errno != 0) __early_abort(__LINE__);
- if (is_AT_SECURE) {
- // If this is a setuid/setgid program, close the security hole described in
- // https://www.freebsd.org/security/advisories/FreeBSD-SA-02:23.stdio.asc
+ // Always ensure that STDIN/STDOUT/STDERR exist. This prevents file
+ // descriptor confusion bugs where a parent process closes
+ // STD*, the exec()d process calls open() for an unrelated reason,
+ // the newly created file descriptor is assigned
+ // 0<=FD<=2, and unrelated code attempts to read / write to the STD*
+ // FDs.
+ // In particular, this can be a security bug for setuid/setgid programs.
+ // For example:
+ // https://www.freebsd.org/security/advisories/FreeBSD-SA-02:23.stdio.asc
+ // However, for robustness reasons, we don't limit these protections to
+ // just security critical executables.
+ //
+ // Init is excluded from these protections unless AT_SECURE is set, as
+ // /dev/null and/or /sys/fs/selinux/null will not be available at
+ // early boot.
+ if ((getpid() != 1) || is_AT_SECURE) {
__nullify_closed_stdio();
+ }
+ if (is_AT_SECURE) {
__sanitize_environment_variables(env);
}