From ddb8ecc4ebf260e4967f57f271d4f5761abeac3e Mon Sep 17 00:00:00 2001
From: "Hongli Lai (Phusion)" <hongli@phusion.nl>
Date: Mon, 7 Dec 2015 12:17:49 +0100
Subject: [PATCH] Fix CVE-2015-7519 header collision vulnerability

---
 CHANGELOG                                 |  1 +
 src/agent/Core/Controller/SendRequest.cpp | 45 ++++++++++++++++++++---
 2 files changed, 40 insertions(+), 6 deletions(-)

diff --git a/CHANGELOG b/CHANGELOG
index 494203acb8..79360788f4 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,6 +1,7 @@
 Release 5.0.22
 --------------
 
+ * Fixes a header collision vulnerability (CVE-2015-7519, medium severity). Please see our blog for detailed vulnerability description and advisory. Thanks to the SUSE security team for reporting this issue.
  * [Apache] Fixes compatibility with Apache 2.4.17's mod_autoindex. Fix contributed by Eric Covener. Closes GH-1642.
  * [Standalone] Passenger Standalone now [accepts configuration options from environment variables](https://www.phusionpassenger.com/library/config/standalone/intro.html). This makes using Passenger Standalone significantly easier on Heroku or on systems that follow the 12-factor principle. Closes GH-1661.
  * [Standalone] The Nginx configuration template has been cleaned up. It is now significantly easier to edit the Nginx configuration template without breaking compatibility with future versions.
diff --git a/src/agent/Core/Controller/SendRequest.cpp b/src/agent/Core/Controller/SendRequest.cpp
index 64d3d9e9de..6da59c2865 100644
--- a/src/agent/Core/Controller/SendRequest.cpp
+++ b/src/agent/Core/Controller/SendRequest.cpp
@@ -204,6 +204,33 @@ Controller::sendBodyToAppWhenAppSinkIdle(Channel *_channel, unsigned int size) {
 	}
 }
 
+static bool
+isAlphaNum(char ch) {
+	return (ch >= '0' && ch <= '9') || (ch >= 'a' && ch <= 'z') || (ch >= 'A' && ch <= 'Z');
+}
+
+/**
+ * For CGI, alphanum headers with optional dashes are mapped to UPP3R_CAS3. This
+ * function can be used to reject non-alphanum/dash headers that would end up with
+ * the same mapping (e.g. upp3r_cas3 and upp3r-cas3 would end up the same, and
+ * potentially collide each other in the receiving application). This is
+ * used to fix CVE-2015-7519.
+ */
+static bool
+containsNonAlphaNumDash(const LString &s) {
+	const LString::Part *part = s.start;
+	while (part != NULL) {
+		for (unsigned int i = 0; i < part->size; i++) {
+			const char start = part->data[i];
+			if (start != '-' && !isAlphaNum(start)) {
+				return true;
+			}
+		}
+		part = part->next;
+	}
+	return false;
+}
+
 static void
 httpHeaderToScgiUpperCase(unsigned char *data, unsigned int size) {
 	static const boost::uint8_t toUpperMap[256] = {
@@ -529,12 +556,18 @@ Controller::constructHeaderForSessionProtocol(Request *req, char * restrict buff
 
 	ServerKit::HeaderTable::Iterator it(req->headers);
 	while (*it != NULL) {
-		if ((it->header->hash == HTTP_CONTENT_LENGTH.hash()
-			|| it->header->hash == HTTP_CONTENT_TYPE.hash()
-			|| it->header->hash == HTTP_CONNECTION.hash())
-		 && (psg_lstr_cmp(&it->header->key, P_STATIC_STRING("content-type"))
-			|| psg_lstr_cmp(&it->header->key, P_STATIC_STRING("content-length"))
-			|| psg_lstr_cmp(&it->header->key, P_STATIC_STRING("connection"))))
+		// This header-skipping is not accounted for in determineHeaderSizeForSessionProtocol(), but
+		// since we are only reducing the size it just wastes some mem bytes.
+		if ((
+				(it->header->hash == HTTP_CONTENT_LENGTH.hash()
+						|| it->header->hash == HTTP_CONTENT_TYPE.hash()
+						|| it->header->hash == HTTP_CONNECTION.hash()
+				) && (psg_lstr_cmp(&it->header->key, P_STATIC_STRING("content-type"))
+						|| psg_lstr_cmp(&it->header->key, P_STATIC_STRING("content-length"))
+						|| psg_lstr_cmp(&it->header->key, P_STATIC_STRING("connection"))
+				)
+			) || containsNonAlphaNumDash(it->header->key)
+		   )
 		{
 			it.next();
 			continue;