Package: release.debian.org
Severity: normal
Tags: wheezy
User: release.debian....@packages.debian.org
Usertags: pu
Hi,
There is another minor security issue in ruby-passenger concerning
insecure usage of temp files.
CVE-2014-1831 and CVE-2014-1832 have been assigned for this issue.
I'd like to fix those by backporting the relevant upstream commits,
see the attached debdiff.
Cheers,
Felix
diff -Nru ruby-passenger-3.0.13debian/debian/changelog ruby-passenger-3.0.13debian/debian/changelog
--- ruby-passenger-3.0.13debian/debian/changelog 2013-10-14 22:43:12.000000000 +0200
+++ ruby-passenger-3.0.13debian/debian/changelog 2014-03-08 19:43:55.000000000 +0100
@@ -1,3 +1,11 @@
+ruby-passenger (3.0.13debian-1+deb7u2) wheezy; urgency=medium
+
+ * Fix CVE-2014-1831 and CVE-2014-1832: insecure use of /tmp.
+ (Closes: #736958)
+ - Backport upstream commits in CVE-2014-1831.patch and CVE-2014-1832.patch
+
+ -- Felix Geyer <fge...@debian.org> Sat, 08 Mar 2014 19:42:03 +0100
+
ruby-passenger (3.0.13debian-1+deb7u1) wheezy; urgency=low
* Fix CVE-2013-2119 and CVE-2013-4136: insecure tmp files usage.
diff -Nru ruby-passenger-3.0.13debian/debian/patches/CVE-2014-1831.patch ruby-passenger-3.0.13debian/debian/patches/CVE-2014-1831.patch
--- ruby-passenger-3.0.13debian/debian/patches/CVE-2014-1831.patch 1970-01-01 01:00:00.000000000 +0100
+++ ruby-passenger-3.0.13debian/debian/patches/CVE-2014-1831.patch 2014-03-08 19:41:41.000000000 +0100
@@ -0,0 +1,100 @@
+From 34b1087870c2bf85ebfd72c30b78577e10ab9744 Mon Sep 17 00:00:00 2001
+From: "Hongli Lai (Phusion)" <hon...@phusion.nl>
+Date: Tue, 28 Jan 2014 19:17:39 +0100
+Subject: [PATCH] Fix low-urgency security vulnerability: writing files to
+ arbitrary directory by hijacking temp directories.
+
+---
+ ext/common/ServerInstanceDir.h | 2 +-
+ ext/common/Utils.cpp | 29 +++++++++++++++++++++++++++++
+ ext/common/Utils.h | 8 +++++++-
+ 4 files changed, 67 insertions(+), 2 deletions(-)
+
+diff --git a/ext/common/ServerInstanceDir.h b/ext/common/ServerInstanceDir.h
+index 136437a..8da3cf3 100644
+--- a/ext/common/ServerInstanceDir.h
++++ b/ext/common/ServerInstanceDir.h
+@@ -213,7 +213,7 @@ class ServerInstanceDir: public noncopyable {
+ * generations no matter what user they're running as.
+ */
+ if (owner) {
+- switch (getFileType(path)) {
++ switch (getFileTypeNoFollowSymlinks(path)) {
+ case FT_NONEXISTANT:
+ createDirectory(path);
+ break;
+diff --git a/ext/common/Utils.cpp b/ext/common/Utils.cpp
+index 1f3dec5..d1db8d6 100644
+--- a/ext/common/Utils.cpp
++++ b/ext/common/Utils.cpp
+@@ -143,6 +143,35 @@
+ }
+ }
+
++FileType
++getFileTypeNoFollowSymlinks(const StaticString &filename) {
++ struct stat buf;
++ int ret;
++
++ ret = lstat(filename.c_str(), &buf);
++ if (ret == 0) {
++ if (S_ISREG(buf.st_mode)) {
++ return FT_REGULAR;
++ } else if (S_ISDIR(buf.st_mode)) {
++ return FT_DIRECTORY;
++ } else if (S_ISLNK(buf.st_mode)) {
++ return FT_SYMLINK;
++ } else {
++ return FT_OTHER;
++ }
++ } else {
++ if (errno == ENOENT) {
++ return FT_NONEXISTANT;
++ } else {
++ int e = errno;
++ string message("Cannot lstat '");
++ message.append(filename);
++ message.append("'");
++ throw FileSystemException(message, e, filename);
++ }
++ }
++}
++
+ void
+ createFile(const string &filename, const StaticString &contents, mode_t permissions, uid_t owner,
+ gid_t group, bool overwrite)
+diff --git a/ext/common/Utils.h b/ext/common/Utils.h
+index 41e6214..5cfaf92 100644
+--- a/ext/common/Utils.h
++++ b/ext/common/Utils.h
+@@ -65,6 +65,8 @@
+ FT_REGULAR,
+ /** A directory. */
+ FT_DIRECTORY,
++ /** A symlink. Only returned by getFileTypeNoFollowSymlinks(), not by getFileType(). */
++ FT_SYMLINK,
+ /** Something else, e.g. a pipe or a socket. */
+ FT_OTHER
+ } FileType;
+@@ -110,7 +112,7 @@ bool fileExists(const StaticString &filename, CachedFileStat *cstat = 0,
+ /**
+ * Check whether 'filename' exists and what kind of file it is.
+ *
+- * @param filename The filename to check.
++ * @param filename The filename to check. It MUST be NULL-terminated.
+ * @param mstat A CachedFileStat object, if you want to use cached statting.
+ * @param throttleRate A throttle rate for cstat. Only applicable if cstat is not NULL.
+ * @return The file type.
+@@ -121,6 +123,10 @@ bool fileExists(const StaticString &filename, CachedFileStat *cstat = 0,
+ */
+ FileType getFileType(const StaticString &filename, CachedFileStat *cstat = 0,
+ unsigned int throttleRate = 0);
++/**
++ * Like getFileType(), but does not follow symlinks.
++ */
++FileType getFileTypeNoFollowSymlinks(const StaticString &filename);
+
+ /**
+ * Create the given file with the given contents, permissions and ownership.
+--
+1.8.5.5
diff -Nru ruby-passenger-3.0.13debian/debian/patches/CVE-2014-1832.patch ruby-passenger-3.0.13debian/debian/patches/CVE-2014-1832.patch
--- ruby-passenger-3.0.13debian/debian/patches/CVE-2014-1832.patch 1970-01-01 01:00:00.000000000 +0100
+++ ruby-passenger-3.0.13debian/debian/patches/CVE-2014-1832.patch 2014-03-08 19:47:38.000000000 +0100
@@ -0,0 +1,145 @@
+From 94428057c602da3d6d34ef75c78091066ecac5c0 Mon Sep 17 00:00:00 2001
+From: "Hongli Lai (Phusion)" <hon...@phusion.nl>
+Date: Wed, 29 Jan 2014 14:19:25 +0100
+Subject: [PATCH] Fix a symlink-related security vulnerability.
+
+The fix in commit 34b10878 and contained a small attack time window in
+between two filesystem operations. This has been fixed.
+---
+ ext/common/ServerInstanceDir.h | 38 ++++++++++++++++++++++----------------
+ ext/common/Utils.cpp | 29 -----------------------------
+ ext/common/Utils.h | 6 ------
+ 4 files changed, 40 insertions(+), 51 deletions(-)
+
+diff --git a/ext/common/ServerInstanceDir.h b/ext/common/ServerInstanceDir.h
+index 8da3cf3..1315de5 100644
+--- a/ext/common/ServerInstanceDir.h
++++ b/ext/common/ServerInstanceDir.h
+@@ -193,6 +193,9 @@ class ServerInstanceDir: public noncopyable {
+
+ void initialize(const string &path, bool owner) {
+ TRACE_POINT();
++ struct stat buf;
++ int ret;
++
+ this->path = path;
+ this->owner = owner;
+
+@@ -212,18 +215,25 @@ class ServerInstanceDir: public noncopyable {
+ * rights though, because we want admin tools to be able to list the available
+ * generations no matter what user they're running as.
+ */
++
++ do {
++ ret = lstat(path.c_str(), &buf);
++ } while (ret == -1 && errno == EAGAIN);
+ if (owner) {
+- switch (getFileTypeNoFollowSymlinks(path)) {
+- case FT_NONEXISTANT:
++ if (ret == 0) {
++ if (S_ISDIR(buf.st_mode)) {
++ verifyDirectoryPermissions(path, buf);
++ } else {
++ throw RuntimeException("'" + path + "' already exists, and is not a directory");
++ }
++ } else if (errno == ENOENT) {
+ createDirectory(path);
+- break;
+- case FT_DIRECTORY:
+- verifyDirectoryPermissions(path);
+- break;
+- default:
+- throw RuntimeException("'" + path + "' already exists, and is not a directory");
++ } else {
++ int e = errno;
++ throw FileSystemException("Cannot lstat '" + path + "'",
++ e, path);
+ }
+- } else if (getFileType(path) != FT_DIRECTORY) {
++ } else if (!S_ISDIR(buf.st_mode)) {
+ throw RuntimeException("Server instance directory '" + path +
+ "' does not exist");
+ }
+@@ -259,14 +269,10 @@ class ServerInstanceDir: public noncopyable {
+ * so that an attacker cannot pre-create a directory with too liberal
+ * permissions.
+ */
+- void verifyDirectoryPermissions(const string &path) {
++ void verifyDirectoryPermissions(const string &path, struct stat &buf) {
+ TRACE_POINT();
+- struct stat buf;
+
+- if (stat(path.c_str(), &buf) == -1) {
+- int e = errno;
+- throw FileSystemException("Cannot stat() " + path, e, path);
+- } else if (buf.st_mode != (S_IFDIR | parseModeString("u=rwx,g=rx,o=rx"))) {
++ if (buf.st_mode != (S_IFDIR | parseModeString("u=rwx,g=rx,o=rx"))) {
+ throw RuntimeException("Tried to reuse existing server instance directory " +
+ path + ", but it has wrong permissions");
+ } else if (buf.st_uid != geteuid() || buf.st_gid != getegid()) {
+diff --git a/ext/common/Utils.cpp b/ext/common/Utils.cpp
+index d1db8d6..1f3dec5 100644
+--- a/ext/common/Utils.cpp
++++ b/ext/common/Utils.cpp
+@@ -143,35 +143,6 @@
+ }
+ }
+
+-FileType
+-getFileTypeNoFollowSymlinks(const StaticString &filename) {
+- struct stat buf;
+- int ret;
+-
+- ret = lstat(filename.c_str(), &buf);
+- if (ret == 0) {
+- if (S_ISREG(buf.st_mode)) {
+- return FT_REGULAR;
+- } else if (S_ISDIR(buf.st_mode)) {
+- return FT_DIRECTORY;
+- } else if (S_ISLNK(buf.st_mode)) {
+- return FT_SYMLINK;
+- } else {
+- return FT_OTHER;
+- }
+- } else {
+- if (errno == ENOENT) {
+- return FT_NONEXISTANT;
+- } else {
+- int e = errno;
+- string message("Cannot lstat '");
+- message.append(filename);
+- message.append("'");
+- throw FileSystemException(message, e, filename);
+- }
+- }
+-}
+-
+ void
+ createFile(const string &filename, const StaticString &contents, mode_t permissions, uid_t owner,
+ gid_t group, bool overwrite)
+diff --git a/ext/common/Utils.h b/ext/common/Utils.h
+index 5cfaf92..a04e507 100644
+--- a/ext/common/Utils.h
++++ b/ext/common/Utils.h
+@@ -65,8 +65,6 @@
+ FT_REGULAR,
+ /** A directory. */
+ FT_DIRECTORY,
+- /** A symlink. Only returned by getFileTypeNoFollowSymlinks(), not by getFileType(). */
+- FT_SYMLINK,
+ /** Something else, e.g. a pipe or a socket. */
+ FT_OTHER
+ } FileType;
+@@ -123,10 +121,6 @@ bool fileExists(const StaticString &filename, CachedFileStat *cstat = 0,
+ */
+ FileType getFileType(const StaticString &filename, CachedFileStat *cstat = 0,
+ unsigned int throttleRate = 0);
+-/**
+- * Like getFileType(), but does not follow symlinks.
+- */
+-FileType getFileTypeNoFollowSymlinks(const StaticString &filename);
+
+ /**
+ * Create the given file with the given contents, permissions and ownership.
+--
+1.8.5.5
diff -Nru ruby-passenger-3.0.13debian/debian/patches/series ruby-passenger-3.0.13debian/debian/patches/series
--- ruby-passenger-3.0.13debian/debian/patches/series 2013-07-21 13:02:52.000000000 +0200
+++ ruby-passenger-3.0.13debian/debian/patches/series 2014-03-08 19:42:00.000000000 +0100
@@ -1,3 +1,5 @@
fix_install_path.patch
CVE-2013-2119.patch
CVE-2013-4136.patch
+CVE-2014-1831.patch
+CVE-2014-1832.patch
