Sebastian Unger wrote:
Why is it trying to open the directory
in the first place?
Security.
Apparently POSIX doesn't allow this level of paranoia for mkdir -p, so I removed
it in the attached Gnulib patch, and this should appear in the next coreutils
release.
A filesystem that doesn't let you read your own directory that you just created
is likely to run into other problems like this -- i.e., the practice may
introduce more security problems than it closes. But I digress.
From 1f869b77a7e1bb871d58d30edd2ea04a9ac04d32 Mon Sep 17 00:00:00 2001
From: Paul Eggert <egg...@cs.ucla.edu>
Date: Tue, 22 Sep 2015 20:04:13 -0700
Subject: [PATCH] savewd: remove SAVEWD_CHDIR_READABLE
It was problematic in the light of file systems that ignore umask.
Problem reported by Sebastian Ungar in: http://bugs.gnu.org/21534
* NEWS: Document this.
* lib/mkancesdirs.c (mkancesdirs): MAKE_DIR now returns 0 if
successful, -1 (setting errno) on failure, rather than something
more complicated than that.
* lib/mkdir-p.c (make_dir_parents):
Do not use SAVEWD_CHDIR_READABLE.
* lib/savewd.c (savewd_chdir):
Remove support for SAVEWD_CHDIR_READABLE.
* lib/savewd.h (SAVEWD_CHDIR_READABLE): Remove.
---
ChangeLog | 13 +++++++++++++
NEWS | 2 ++
lib/mkancesdirs.c | 24 ++++++------------------
lib/mkdir-p.c | 4 +---
lib/savewd.c | 2 +-
lib/savewd.h | 9 ++-------
6 files changed, 25 insertions(+), 29 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 62458b2..1bcc6f1 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,18 @@
2015-09-22 Paul Eggert <egg...@cs.ucla.edu>
+ savewd: remove SAVEWD_CHDIR_READABLE
+ It was problematic in the light of file systems that ignore umask.
+ Problem reported by Sebastian Ungar in: http://bugs.gnu.org/21534
+ * NEWS: Document this.
+ * lib/mkancesdirs.c (mkancesdirs): MAKE_DIR now returns 0 if
+ successful, -1 (setting errno) on failure, rather than something
+ more complicated than that.
+ * lib/mkdir-p.c (make_dir_parents):
+ Do not use SAVEWD_CHDIR_READABLE.
+ * lib/savewd.c (savewd_chdir):
+ Remove support for SAVEWD_CHDIR_READABLE.
+ * lib/savewd.h (SAVEWD_CHDIR_READABLE): Remove.
+
c-ctype: port better to EBCDIC
Problems reported by Daniel Richard G. in
http://lists.gnu.org/archive/html/bug-gnulib/2015-09/msg00020.html
diff --git a/NEWS b/NEWS
index b294edd..b54a992 100644
--- a/NEWS
+++ b/NEWS
@@ -42,6 +42,8 @@ User visible incompatible changes
Date Modules Changes
+2015-09-22 savewd SAVEWD_CHDIR_READABLE constant removed.
+
2015-07-24 fprintftime Exported functions' time zone arguments are now of
strftime type timezone_t (with NULL denoting UTC) instead of
type int (with nonzero denoting UTC). These
diff --git a/lib/mkancesdirs.c b/lib/mkancesdirs.c
index 91ed694..2747d4a 100644
--- a/lib/mkancesdirs.c
+++ b/lib/mkancesdirs.c
@@ -42,11 +42,9 @@
Create any ancestor directories that don't already exist, by
invoking MAKE_DIR (FILE, COMPONENT, MAKE_DIR_ARG). This function
- should return 0 if successful and the resulting directory is
- readable, 1 if successful but the resulting directory might not be
- readable, -1 (setting errno) otherwise. If COMPONENT is relative,
- it is relative to the temporary working directory, which may differ
- from *WD.
+ should return 0 if successful, -1 (setting errno) otherwise. If
+ COMPONENT is relative, it is relative to the temporary working
+ directory, which may differ from *WD.
Ordinarily MAKE_DIR is executed with the working directory changed
to reflect the already-made prefix, and mkancesdirs returns with
@@ -112,20 +110,10 @@ mkancesdirs (char *file, struct savewd *wd,
if (sep - component == 2
&& component[0] == '.' && component[1] == '.')
made_dir = false;
+ else if (make_dir (file, component, make_dir_arg) < 0)
+ make_dir_errno = errno;
else
- switch (make_dir (file, component, make_dir_arg))
- {
- case -1:
- make_dir_errno = errno;
- break;
-
- case 0:
- savewd_chdir_options |= SAVEWD_CHDIR_READABLE;
- /* Fall through. */
- case 1:
- made_dir = true;
- break;
- }
+ made_dir = true;
if (made_dir)
savewd_chdir_options |= SAVEWD_CHDIR_NOFOLLOW;
diff --git a/lib/mkdir-p.c b/lib/mkdir-p.c
index e55b1dd..e518272 100644
--- a/lib/mkdir-p.c
+++ b/lib/mkdir-p.c
@@ -136,9 +136,7 @@ make_dir_parents (char *dir,
announce (dir, options);
preserve_existing = (keep_owner & keep_special_mode_bits
& umask_must_be_ok);
- savewd_chdir_options |=
- (SAVEWD_CHDIR_NOFOLLOW
- | (mode & S_IRUSR ? SAVEWD_CHDIR_READABLE : 0));
+ savewd_chdir_options |= SAVEWD_CHDIR_NOFOLLOW;
}
else
{
diff --git a/lib/savewd.c b/lib/savewd.c
index 16f56a8..f835074 100644
--- a/lib/savewd.c
+++ b/lib/savewd.c
@@ -116,7 +116,7 @@ savewd_chdir (struct savewd *wd, char const *dir, int
options,
open_result[1] = errno;
}
- if (fd < 0 && (errno != EACCES || (options & SAVEWD_CHDIR_READABLE)))
+ if (fd < 0 && errno != EACCES)
result = -1;
}
diff --git a/lib/savewd.h b/lib/savewd.h
index bb442b3..69b3718 100644
--- a/lib/savewd.h
+++ b/lib/savewd.h
@@ -82,20 +82,15 @@ savewd_init (struct savewd *wd)
}
-/* Options for savewd_chdir. */
+/* Options for savewd_chdir. Can be ORed together. */
enum
{
/* Do not follow symbolic links, if supported. */
SAVEWD_CHDIR_NOFOLLOW = 1,
- /* The directory should be readable, so fail if it happens to be
- discovered that the directory is not readable. (Unreadable
- directories are not necessarily diagnosed, though.) */
- SAVEWD_CHDIR_READABLE = 2,
-
/* Do not chdir if the directory is readable; simply succeed
without invoking chdir if the directory was opened. */
- SAVEWD_CHDIR_SKIP_READABLE = 4
+ SAVEWD_CHDIR_SKIP_READABLE = 2
};
/* Change the directory, and if successful, record into *WD the fact
--
2.1.0
