Re: [PATCH] setup: avoid unconditional open with write flags
- From
brian m. carlson <sandals@crustytoothpaste.net>
- Date
- Dec 5, 2022, 22:13 UTC
- Message-ID
- <Y45tGLaJwg3Lt5uJ@tapette.crustytoothpaste.net>
- In-Reply-To
- <20221205190019.52829-1-cgzones@googlemail.com>
On 2022-12-05 at 19:00:19, Christian Göttsche wrote:
Show 17 quoted lines
> 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.I don't think this patch makes anything worse, and so I think it should be fine as it is.
_However_, I will say that `git status` is not a read-only command because it can write the index, and we aren't, in general, going to be able to promise that any portion of Git will work with only O_RDONLY file descriptors. I suspect such a sandbox is going to result in a highly broken Git in general, and so it wouldn't be a good idea.
-- brian m. carlson (he/him or they/them) Toronto, Ontario, CA