[PATCH 1/6] add open_nofollow() helper

From: Jeff King <peff@peff.net>
Date: Feb 16, 2021, 14:44 UTC
Message-ID: <YCvaRpQs99TBaCrQ@coredump.intra.peff.net>
In-Reply-To: <YCvaJg8o882IqNnx@coredump.intra.peff.net>

Some callers of open() would like to use O_NOFOLLOW, but it is not available on all platforms. Let's abstract this into a helper function so we can provide system-specific implementations.

Some light web-searching reveals that we might be able to get something similar on Windows using FILE_FLAG_OPEN_REPARSE_POINT. I didn't dig into this further.

For other systems without O_NOFOLLOW or any equivalent, we have two options for fallback:

  - we can just open anyway, following symlinks; this may have security
    implications (e.g., following untrusted in-tree symlinks)

  - we can determine whether the path is a symlink with lstat().

    This is slower (two syscalls instead of one), but that may be
    acceptable for infrequent uses like looking up .gitattributes files
    (especially because we can get away with a single syscall for the
    common case of ENOENT).

    It's also racy, but should be sufficient for our needs (we are
    worried about in-tree symlinks that we ourselves would have
    previously created). We could make it non-racy at the cost of making
    it even slower, by doing an fstat() on the opened descriptor and
    comparing the dev/ino fields to the original lstat().

This patch implements the lstat() option in its slightly-faster racy form.

Signed-off-by: Jeff King <peff@peff.net>
---
 git-compat-util.h |  7 +++++++
 wrapper.c         | 16 ++++++++++++++++
 2 files changed, 23 insertions(+)

diff --git a/git-compat-util.h b/git-compat-util.h
index 838246289c..fd99eaeb6d 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -1231,6 +1231,13 @@ int access_or_die(const char *path, int mode, unsigned flag);
 /* Warn on an inaccessible file if errno indicates this is an error */
 int warn_on_fopen_errors(const char *path);
 
+/*
+ * Open with O_NOFOLLOW, or equivalent. Note that the fallback equivalent
+ * may be racy. Do not use this as protection against an attacker who can
+ * simultaneously create paths.
+ */
+int open_nofollow(const char *path, int flags);
+
 #if !defined(USE_PARENS_AROUND_GETTEXT_N) && defined(__GNUC__)
 #define USE_PARENS_AROUND_GETTEXT_N 1
 #endif
diff --git a/wrapper.c b/wrapper.c
index bcda41e374..563ad590df 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -678,3 +678,19 @@ int is_empty_or_missing_file(const char *filename)
 
 	return !st.st_size;
 }
+
+int open_nofollow(const char *path, int flags)
+{
+#ifdef O_NOFOLLOW
+	return open(path, flags | O_NOFOLLOW);
+#else
+	struct stat st;
+	if (lstat(path, &st) < 0)
+		return -1;
+	if (S_ISLNK(st.st_mode)) {
+		errno = ELOOP;
+		return -1;
+	}
+	return open(path, flags);
+#endif
+}

-- 
2.30.1.986.gd86016a168