Remove constructor, do not verify fd with fcntl

By the logic of the previous commit, it's not our job to verify that the
user's whacked out set-id configuration is sane, only that the path they
passed does not refer to a different file now than it did when ape found
it. This is generally true of the files under /dev/fd in our case.

So we just check that the file is in /dev/fd/, that it starts with not a
'.', and that it does not contain a '/'.

Since we don't care whether there actually is an fd by that name, we can
do the check when called and save a constructor.
This commit is contained in:
Jōshin 2023-12-18 15:50:04 -05:00
parent ba2b8b8c68
commit a5184ba1ef
No known key found for this signature in database

View file

@ -23,7 +23,6 @@
#include "libc/cosmo.h" #include "libc/cosmo.h"
#include "libc/dce.h" #include "libc/dce.h"
#include "libc/errno.h" #include "libc/errno.h"
#include "libc/fmt/conv.h"
#include "libc/fmt/libgen.h" #include "libc/fmt/libgen.h"
#include "libc/intrin/getenv.internal.h" #include "libc/intrin/getenv.internal.h"
#include "libc/serialize.h" #include "libc/serialize.h"
@ -33,7 +32,6 @@
#include "libc/runtime/runtime.h" #include "libc/runtime/runtime.h"
#include "libc/str/str.h" #include "libc/str/str.h"
#include "libc/sysv/consts/at.h" #include "libc/sysv/consts/at.h"
#include "libc/sysv/consts/f.h"
#include "libc/sysv/consts/ok.h" #include "libc/sysv/consts/ok.h"
#define CTL_KERN 1 #define CTL_KERN 1
@ -50,8 +48,6 @@ static struct {
} u; } u;
} g_prog; } g_prog;
static bool g_ape_loaded;
static inline int IsAlpha(int c) { static inline int IsAlpha(int c) {
return ('A' <= c && c <= 'Z') || ('a' <= c && c <= 'z'); return ('A' <= c && c <= 'Z') || ('a' <= c && c <= 'z');
} }
@ -109,29 +105,6 @@ static int TryPath(const char *q, int com) {
return 0; return 0;
} }
__attribute__((__constructor__)) static void setugidNameCheck(void) {
char *end;
unsigned long rc;
if (__program_executable_name) {
g_ape_loaded = true;
if (issetugid()) {
/* we are running as a set-id interpreter script. this is highly unusual.
it means either someone installed their ape loader set-id, or they are
running a system that supports secure set-id interpreter scripts via a
/dev/fd/ path. check for the latter and allow that. otherwise, set the
__program_executable_name to NULL since there is an unavoidable TOCTOU
problem between the kernel and the loader. */
if ((!IsNetbsd() && !IsOpenbsd() && !IsXnu()) /* any others? */ ||
0 != strncmp(DEV_FD, __program_executable_name, sizeof(DEV_FD) - 1) ||
(rc = strtoul(__program_executable_name + sizeof(DEV_FD) - 1, &end,
10)) > INT_MAX ||
*end || rc < 3 || fcntl((int)rc, F_GETFD) == -1) {
__program_executable_name = NULL;
}
}
}
}
static inline void InitProgramExecutableNameImpl(void) { static inline void InitProgramExecutableNameImpl(void) {
size_t n; size_t n;
ssize_t got; ssize_t got;
@ -161,13 +134,22 @@ static inline void InitProgramExecutableNameImpl(void) {
return; return;
} }
if (g_ape_loaded) { // see if the loader passed us a path.
/* the loader passed us a path. it may be relative. */ if (__program_executable_name) {
if (!__program_executable_name) { if (issetugid()) {
/* we rejected the loader path because we are running set-id and it /* we are running as a set-id interpreter script. this is highly unusual.
looked insecure. use the empty string. */ it means either someone installed their ape loader set-id, or they are
running a system that supports secure set-id interpreter scripts via a
/dev/fd/ path. check for the latter and allow that. otherwise, use the
empty string to obviate the TOCTOU problem between loader and binary.
*/
if ((!IsNetbsd() && !IsOpenbsd() && !IsXnu()) /* any others? */ ||
0 != strncmp(DEV_FD, __program_executable_name, sizeof(DEV_FD) - 1) ||
__program_executable_name[sizeof(DEV_FD) - 1] == '.' ||
strchr(__program_executable_name + sizeof(DEV_FD) - 1, '/')) {
goto UseEmpty; goto UseEmpty;
} }
}
if (*__program_executable_name == '/') { if (*__program_executable_name == '/') {
return; return;
} }