From 22aa53b31439fc3eedf1ffc7a40d494db123ac9d Mon Sep 17 00:00:00 2001 From: Wietse Venema Date: Thu, 14 Aug 2008 00:00:00 -0500 Subject: [PATCH] postfix-2.5.4 --- postfix/HISTORY | 19 +++++++++++++++++++ postfix/src/global/mail_version.h | 4 ++-- postfix/src/util/safe_open.c | 21 +++++++++++++++++++-- postfix/src/util/vstring.c | 1 + 4 files changed, 41 insertions(+), 4 deletions(-) diff --git a/postfix/HISTORY b/postfix/HISTORY index 5bdc156d7..d4b00cf0f 100644 --- a/postfix/HISTORY +++ b/postfix/HISTORY @@ -14411,3 +14411,22 @@ Apologies for any names omitted. by the recipient. Requested by Sebastian Krahmer, SuSE. Specify "strict_mailbox_ownership=no" to ignore ownership discrepancies. 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 7e7bcf5d8..4207d1bb7 100644 --- a/postfix/src/global/mail_version.h +++ b/postfix/src/global/mail_version.h @@ -20,8 +20,8 @@ * 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_VERSION_NUMBER "2.5.3" +#define MAIL_RELEASE_DATE "20080814" +#define MAIL_VERSION_NUMBER "2.5.4" #ifdef SNAPSHOT # define MAIL_VERSION_DATE "-" MAIL_RELEASE_DATE 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 */