From acb84351c93692c1a41ae2ca7c9de17292196c3e Mon Sep 17 00:00:00 2001 From: Wietse Venema Date: Thu, 14 Aug 2008 00:00:00 -0500 Subject: [PATCH] postfix-2.6-20080814 --- postfix/HISTORY | 20 ++++++++++++++++++++ postfix/src/global/mail_version.h | 2 +- postfix/src/util/safe_open.c | 21 +++++++++++++++++++-- postfix/src/util/vstring.c | 1 + 4 files changed, 41 insertions(+), 3 deletions(-) diff --git a/postfix/HISTORY b/postfix/HISTORY index 310cc6ad0..3cd0526eb 100644 --- a/postfix/HISTORY +++ b/postfix/HISTORY @@ -14549,3 +14549,23 @@ Apologies for any names omitted. 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. + diff --git a/postfix/src/global/mail_version.h b/postfix/src/global/mail_version.h index 861981974..23eae8261 100644 --- a/postfix/src/global/mail_version.h +++ b/postfix/src/global/mail_version.h @@ -20,7 +20,7 @@ * 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 diff --git a/postfix/src/util/safe_open.c b/postfix/src/util/safe_open.c index c825493ca..9cf5d0263 100644 --- a/postfix/src/util/safe_open.c +++ b/postfix/src/util/safe_open.c @@ -83,6 +83,7 @@ #include #include #include +#include #include /* safe_open_exist - open existing file */ @@ -138,13 +139,29 @@ static VSTREAM *safe_open_exist(const char *path, int flags, * 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 diff --git a/postfix/src/util/vstring.c b/postfix/src/util/vstring.c index df65a69bc..2a3234397 100644 --- a/postfix/src/util/vstring.c +++ b/postfix/src/util/vstring.c @@ -624,6 +624,7 @@ VSTRING *vstring_sprintf_prepend(VSTRING *vp, const char *format,...) 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 */