Re: [PATCH] setup: avoid unconditional open with write flags

["brian m. carlson" <sandals@crustytoothpaste.net>]

[-- Attachment #1: Type: text/plain, Size: 1568 bytes --]

On 2022-12-05 at 19:00:19, Christian Göttsche wrote:
> 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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 263 bytes --]