Am 05.12.22 um 20:00 schrieb Christian Göttsche:
> Commit 57f5d52a942 ("common-main: call sanitize_stdfds()") added the
> sanitization for standard file descriptors (stdin, stdout, stderr) to
> all binaries. The lead to all binaries unconditionally opening
> /dev/null with the flag O_RDWR (read and write). Most of the time the
> standard file descriptors should be set up properly and the sanitization
> ends up doing nothing.
>
> There are many non modifying git operations, like `git status` or `git
> stash list`, which might be called by a parent to gather information
> about the repository. That parent might run under a seccomp filter to
> avoid accidental modification or unwanted command execution on memory
> corruptions. As part of that seccomp filter open(2) and openat(2) might
> be only allowed in read-only mode (O_RDONLY), thus preventing git's
> sanitation and stopping the application.
>
> Check before opening /dev/null to populate a possible non-present
> standard file descriptor if actually any is missing.
>
> Signed-off-by: Christian Göttsche <cgzones@xxxxxxxxxxxxxx>
> ---
> Alternatively one could add a command line argument
> (`--no-stdfd-sanitization`).
> ---
> setup.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/setup.c b/setup.c
> index cefd5f6..2af7170 100644
> --- a/setup.c
> +++ b/setup.c
> @@ -1669,7 +1669,14 @@ const char *resolve_gitdir_gently(const char *suspect, int *return_error_code)
> /* if any standard file descriptor is missing open it to /dev/null */
> void sanitize_stdfds(void)
> {
> - int fd = xopen("/dev/null", O_RDWR);
> + int fd;
> +
> + if (fcntl(0, F_GETFD) != -1 &&
> + fcntl(1, F_GETFD) != -1 &&
> + fcntl(2, F_GETFD) != -1)
> + return;
> +
> + fd = xopen("/dev/null", O_RDWR);
> while (fd < 2)
> fd = xdup(fd);
> if (fd > 2)
If read-only access is allowed, how about this?
diff --git a/setup.c b/setup.c
index cefd5f63c4..0f52c51759 100644
--- a/setup.c
+++ b/setup.c
@@ -1669,7 +1669,12 @@ const char *resolve_gitdir_gently(const char *suspect, int *return_error_code)
/* if any standard file descriptor is missing open it to /dev/null */
void sanitize_stdfds(void)
{
- int fd = xopen("/dev/null", O_RDWR);
+ int fd = xopen("/dev/null", O_RDONLY);
+ if (fd > 0)
+ close(fd);
+ if (fd > 2)
+ return;
+ fd = xopen("/dev/null", O_WRONLY);
while (fd < 2)
fd = xdup(fd);
if (fd > 2)
Requires an extra open/close pair if fd 0 is already open, but no extra
syscalls if 0, 1 and 2 are all open.
Can opening /dev/null (or NUL on Windows) multiple times instead of dup'ing
cause issues? Can we e.g. lock ourselves out?
René
