Paranoia: defer delivery when a mailbox file is not owned
by the recipient. Sebastian Krahmer, SuSE. Files:
local/mailbox.c, virtual/mailbox.c.
+
+20080804
+
+ Bugfix: dangling pointer in vstring_sprintf_prepend().
+ File: util/vstring.c.
+
+20080814
+
+ Security: some systems have changed their link() semantics,
+ and will hardlink a symlink, contrary to POSIX and XPG4.
+ Sebastian Krahmer, SuSE. File: util/safe_open.c.
+
+ The solution introduces the following incompatible change:
+ when the target of mail delivery is a symlink, the parent
+ directory of that symlink must now be writable by root only
+ (in addition to the already existing requirement that the
+ symlink itself is owned by root). This change will break
+ legitimate configurations that deliver mail to a symbolic
+ link in a directory with less restrictive permissions.
+
* Patches change both the patchlevel and the release date. Snapshots have no
* patchlevel; they change the release date only.
*/
-#define MAIL_RELEASE_DATE "20080726"
+#define MAIL_RELEASE_DATE "20080814"
#define MAIL_VERSION_NUMBER "2.6"
#ifdef SNAPSHOT
#include <msg.h>
#include <vstream.h>
#include <vstring.h>
+#include <stringops.h>
#include <safe_open.h>
/* safe_open_exist - open existing file */
* for symlinks owned by root. NEVER, NEVER, make exceptions for symlinks
* owned by a non-root user. This would open a security hole when
* delivering mail to a world-writable mailbox directory.
+ *
+ * Sebastian Krahmer of SuSE brought to my attention that some systems have
+ * changed their semantics of link(symlink, newpath), such that the
+ * result is a hardlink to the symlink. For this reason, we now also
+ * require that the symlink's parent directory is writable only by root.
*/
else if (lstat(path, &lstat_st) < 0) {
vstring_sprintf(why, "file status changed unexpectedly: %m");
errno = EPERM;
} else if (S_ISLNK(lstat_st.st_mode)) {
- if (lstat_st.st_uid == 0)
- return (fp);
+ if (lstat_st.st_uid == 0) {
+ VSTRING *parent_buf = vstring_alloc(100);
+ const char *parent_path = sane_dirname(parent_buf, path);
+ struct stat parent_st;
+ int parent_ok;
+
+ parent_ok = (stat(parent_path, &parent_st) == 0 /* not lstat */
+ && parent_st.st_uid == 0
+ && (parent_st.st_mode & (S_IWGRP | S_IWOTH)) == 0);
+ vstring_free(parent_buf);
+ if (parent_ok)
+ return (fp);
+ }
vstring_sprintf(why, "file is a symbolic link");
errno = EPERM;
} else if (fstat_st->st_dev != lstat_st.st_dev
result_len = VSTRING_LEN(vp);
/* Construct: old|new|old|free */
+ VSTRING_SPACE(vp, old_len);
vstring_memcat(vp, vstring_str(vp), old_len);
/* Construct: new|old|free */