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@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@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@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
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 
<[email protected]>
+Maintainer: Ubuntu Developers <[email protected]>
+XSBC-Original-Maintainer: Debian Ruby Extras Maintainers 
<[email protected]>
 Uploaders: Filipe Lautert <[email protected]>, Micah Anderson 
<[email protected]>, David Moreno <[email protected]>
 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 <[email protected]>  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 <[email protected]>  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 <[email protected]>  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);
                        }
                }
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 <[email protected]>  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);
                        }
                }
_______________________________________________
Pkg-ruby-extras-maintainers mailing list
[email protected]
http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/pkg-ruby-extras-maintainers

Reply via email to