[DRE-maint] Bug#812103: Patch

Trent Lloyd trent.lloyd at canonical.com
Wed Jul 6 00:13:08 UTC 2016


I fixed this issue in Ubuntu Precise (
https://bugs.launchpad.net/ubuntu/+source/passenger/+bug/1575220), sharing
the fix here however this fix was uploaded under the Squeeze LTS project
that concluded in February so I am guessing this may never be uploaded in
Debian.

The current patch modifies the addHeader() function itself to perform the
check, this is invalid because this function is used internally to setup
many headers from the environment such as the standard CGI HTTP_HOST,
REQUEST_URI, etc.

The correct patch should only abort adding headers from the HTTP request.

The upstream patch/source for Passenger 5 was quite different to v2 here,
however the upstream patch for Passenger 4 (
https://github.com/phusion/passenger/commit/c04590871ca0878d4d3ac1220c5a554b049056b4)
was very similar and I have backported this fix to precise in the attached
debdiff. I have not backported the nginx part, it was not done originally.

Attaching two versions of my diff against precise, one is against the
current broken version and the other is against the original version which
makes it easier to see what the complete content of the fix now is.

Patch Testing:
 ** No Patch **
lathiat at ubuntu:~/src/lp1575220$ curl -s -H "X-Test-Dash-Header: Yes" -H
"X_TEST_UNDERSCORE_HEADER: Yes" http://10.48.134.78/|grep -i test
   "HTTP_X_TEST_UNDERSCORE_HEADER"=>"Yes",
   "HTTP_X_TEST_DASH_HEADER"=>"Yes",

 ** Broken Patch **
lathiat at ubuntu:~/src/lp1575220$ curl -s -H "X-Test-Dash-Header: Yes" -H
"X_Test_Underscore_header: Yes" http://10.48.134.78/|grep -i test

 ** New Proposed Patch **
lathiat at ubuntu:~/src/lp1575220$ curl -s -H "X-Test-Dash-Header: Yes" -H
"X_Test_Underscore_header: Yes" http://10.48.134.78/|grep -i test
   "HTTP_X_TEST_DASH_HEADER"=>"Yes",

Cheers,
Trent
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.alioth.debian.org/pipermail/pkg-ruby-extras-maintainers/attachments/20160706/2b6bc313/attachment-0001.html>
-------------- next part --------------
diff -u passenger-2.2.11debian/debian/control passenger-2.2.11debian/debian/control
--- passenger-2.2.11debian/debian/control
+++ passenger-2.2.11debian/debian/control
@@ -1,7 +1,8 @@
 Source: passenger
 Section: web
 Priority: optional
-Maintainer: Debian Ruby Extras Maintainers <pkg-ruby-extras-maintainers at lists.alioth.debian.org>
+Maintainer: Ubuntu Developers <ubuntu-devel-discuss at lists.ubuntu.com>
+XSBC-Original-Maintainer: Debian Ruby Extras Maintainers <pkg-ruby-extras-maintainers at lists.alioth.debian.org>
 Uploaders: Filipe Lautert <filipe at debian.org>, Micah Anderson <micah at debian.org>, David Moreno <damog at debian.org>
 Build-Depends: debhelper (>= 7), apache2-mpm-worker | apache2-mpm, apache2-threaded-dev, libapr1-dev,
  rubygems (>= 1.2), debhelper (>= 5.0.44), ruby-dev, doxygen, asciidoc (>= 8.2), graphviz, rake,
diff -u passenger-2.2.11debian/debian/changelog passenger-2.2.11debian/debian/changelog
--- passenger-2.2.11debian/debian/changelog
+++ passenger-2.2.11debian/debian/changelog
@@ -1,3 +1,33 @@
+passenger (2.2.11debian-2+deb6u1ubuntu12.04.2) precise-security; urgency=medium
+
+  * Fix for regression introduced in previous CVE-2015-7519
+    fix.  All HTTP headers were dropped from the request
+    which broke all applications.  Backport the upstream
+    fix from commit c04590871ca0878d4d3ac1220c5a554b049056b4
+    for Apache2 only (LP: #1575220)
+
+ -- Trent Lloyd <trent.lloyd at canonical.com>  Tue, 05 Jul 2016 00:42:47 +0800
+
+passenger (2.2.11debian-2+deb6u1ubuntu12.04.1) precise-security; urgency=medium
+
+  * fake sync from Debian
+
+ -- Steve Beattie <sbeattie at ubuntu.com>  Mon, 25 Apr 2016 16:38:03 -0700
+
+passenger (2.2.11debian-2+deb6u1) squeeze-lts; urgency=high
+
+  * Non-maintainer upload by the Squeeze LTS Team.
+  * CVE-2015-7519
+    agent/Core/Controller/SendRequest.cpp in Phusion Passenger
+    before 4.0.60 and 5.0.x before 5.0.22, when used in Apache
+    integration mode or in standalone mode without a filtering
+    proxy, allows remote attackers to spoof headers passed to
+    applications by using an _ (underscore) character instead
+    of a - (dash) character in an HTTP header, as demonstrated
+    by an X_User header.
+ 
+ -- Thorsten Alteholz <debian at alteholz.de>  Mon, 28 Jan 2016 18:03:02 +0100
+
 passenger (2.2.11debian-2) unstable; urgency=low
 
   [Laurent Arnoud]
only in patch2:
unchanged:
--- passenger-2.2.11debian.orig/ext/apache2/Hooks.cpp
+++ passenger-2.2.11debian/ext/apache2/Hooks.cpp
@@ -786,6 +786,18 @@
 		}
 	}
 	
+	// Renamed upstream function contains_non_alphanumdash from commit c04590871ca0878d4d3ac1220c5a554b049056b4
+	// because the return values were confusingly opposite to what the name suggested.  Used for CVE-2015-7519 fix.
+	bool contains_alphanumdash_only(const char *current) {
+		while (*current != '\0') {
+			if (!apr_isalnum(*current) && *current != '-') {
+				return false;
+			}
+			current++;
+		}
+		return true;
+	}
+
 	apr_status_t sendHeaders(request_rec *r, DirConfig *config, Application::SessionPtr &session, const char *baseURI) {
 		apr_table_t *headers;
 		headers = apr_table_make(r->pool, 40);
@@ -847,7 +859,7 @@
 		hdrs_arr = apr_table_elts(r->headers_in);
 		hdrs = (apr_table_entry_t *) hdrs_arr->elts;
 		for (i = 0; i < hdrs_arr->nelts; ++i) {
-			if (hdrs[i].key) {
+			if (hdrs[i].key && contains_alphanumdash_only(hdrs[i].key)) {
 				addHeader(headers, http2env(r->pool, hdrs[i].key), hdrs[i].val);
 			}
 		}
-------------- next part --------------
diff -u passenger-2.2.11debian/debian/changelog passenger-2.2.11debian/debian/changelog
--- passenger-2.2.11debian/debian/changelog
+++ passenger-2.2.11debian/debian/changelog
@@ -1,3 +1,13 @@
+passenger (2.2.11debian-2+deb6u1ubuntu12.04.2) precise-security; urgency=medium
+
+  * Fix for regression introduced in previous CVE-2015-7519
+    fix.  All HTTP headers were dropped from the request
+    which broke all applications.  Backport the upstream
+    fix from commit c04590871ca0878d4d3ac1220c5a554b049056b4
+    for Apache2 only (LP: #1575220)
+
+ -- Trent Lloyd <trent.lloyd at canonical.com>  Tue, 05 Jul 2016 00:42:47 +0800
+
 passenger (2.2.11debian-2+deb6u1ubuntu12.04.1) precise-security; urgency=medium
 
   * fake sync from Debian
diff -u passenger-2.2.11debian/ext/apache2/Hooks.cpp passenger-2.2.11debian/ext/apache2/Hooks.cpp
--- passenger-2.2.11debian/ext/apache2/Hooks.cpp
+++ passenger-2.2.11debian/ext/apache2/Hooks.cpp
@@ -779,37 +779,25 @@
 	char *lookupEnv(request_rec *r, const char *name) {
 		return lookupName(r->subprocess_env, name);
 	}
-
-	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 char *s) {
-		size_t len = strlen(s);
-		for (size_t i = 0; i < len; i++) {
-			const char start = s[i];
-			if (start != '-' && !isAlphaNum(start)) {
-				return true;
-			}
-		}
-		return false;
-	}
 	
 	void inline addHeader(apr_table_t *table, const char *name, const char *value) {
-		if (name != NULL && value != NULL && !containsNonAlphaNumDash(name)) {
+		if (name != NULL && value != NULL) {
 			apr_table_addn(table, name, value);
 		}
 	}
 	
+	// Renamed upstream function contains_non_alphanumdash from commit c04590871ca0878d4d3ac1220c5a554b049056b4
+	// because the return values were confusingly opposite to what the name suggested.  Used for CVE-2015-7519 fix.
+	bool contains_alphanumdash_only(const char *current) {
+		while (*current != '\0') {
+			if (!apr_isalnum(*current) && *current != '-') {
+				return false;
+			}
+			current++;
+		}
+		return true;
+	}
+
 	apr_status_t sendHeaders(request_rec *r, DirConfig *config, Application::SessionPtr &session, const char *baseURI) {
 		apr_table_t *headers;
 		headers = apr_table_make(r->pool, 40);
@@ -871,7 +859,7 @@
 		hdrs_arr = apr_table_elts(r->headers_in);
 		hdrs = (apr_table_entry_t *) hdrs_arr->elts;
 		for (i = 0; i < hdrs_arr->nelts; ++i) {
-			if (hdrs[i].key) {
+			if (hdrs[i].key && contains_alphanumdash_only(hdrs[i].key)) {
 				addHeader(headers, http2env(r->pool, hdrs[i].key), hdrs[i].val);
 			}
 		}


More information about the Pkg-ruby-extras-maintainers mailing list