Re: CVE-2015-7519
On 26 April 2016 at 10:27, Linus van Geuns wrote: > On Tue, Apr 26, 2016 at 10:08 AM, Raphael Geissert wrote: >> On 19 February 2016 at 09:35, Linus van Geuns wrote: >>> On Thu, Feb 18, 2016 at 8:35 PM, Thorsten Alteholz >>> wrote: >>>> On irc you wrote: >>>> 15:05 < Nirkus> have some old redmine running on squeeze-lts (yeah..) and >>>> since the update yesterday the following redmine code bails out with >>>> "private method `split' called for nil:NilClass" at the following line: >>>> 15:06 < Nirkus> @env['QUERY_STRING'].present? ? @env['QUERY_STRING'] : >>>> (@env['REQUEST_URI'].split('?', 2)[1] || '') >>>> >>>> In CVE-2015-7519[1] it was detected, that it is possible to obtain >>>> unauthorized access if you send http variables with "_" instead of "-". >>>> More information can be found here[2]. As a solution it was proposed to >>>> simply filter out all variables containing an "_". This was already done >>>> in mod_cgi of apache[3] and now I applied a similar patch to >>>> libapache2-mod-passenger as well. >>>> >>>> Unfortunately there seems to be software that relies on underscores in >>>> variable names. So if you need such variables you might want to use the >>>> workaround for apache, described in[2]. >>> >>> I am only scratching the surface of Ruby, Passenger, Rack/Rails and >>> Redminde, so corrections and clarifications welcome. :) >>> >> [...] >>> >>> I am not sure whether REQUEST_URI and QUERY_STRING are actually passed >>> as per-request env. variables by Passenger or added to the env hash by >>> Rack/Rails. >>> Still, this looks like a regression to me, since it removes previously >>> available variables, which should not be in scope of CVE-2015-7519. >> >> It is a regression, there's no way for applications using >> mod_passenger to work after the latest update. Not only did the update >> switch to a native package and drop some documentation, but it broke >> the module. >> >> Granted, the package is safer now that it doesn't work. > > Yeah, granted > > "We" are no longer affected by this regression since the affected > Redmine instance has been migrated to a current release running on > Debian jessie. > So, thank you for the incentive to do the right thing. FWIW, attached patch should do it. It can be applied on top of the non-LTS version of passenger. Cheers, -- Raphael Geissert Index: passenger-2.2.11debian/ext/apache2/Hooks.cpp === --- passenger-2.2.11debian.orig/ext/apache2/Hooks.cpp 2016-04-26 11:29:20.620915082 +0200 +++ passenger-2.2.11debian/ext/apache2/Hooks.cpp 2016-04-26 14:20:05.944237319 +0200 @@ -779,6 +779,30 @@ private: 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) { @@ -847,7 +871,7 @@ private: 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 && !containsNonAlphaNumDash(hdrs[i].key)) { addHeader(headers, http2env(r->pool, hdrs[i].key), hdrs[i].val); } }
Re: CVE-2015-7519
Hi, On 19 February 2016 at 09:35, Linus van Geuns wrote: > On Thu, Feb 18, 2016 at 8:35 PM, Thorsten Alteholz wrote: >> On irc you wrote: >> 15:05 < Nirkus> have some old redmine running on squeeze-lts (yeah..) and >> since the update yesterday the following redmine code bails out with >> "private method `split' called for nil:NilClass" at the following line: >> 15:06 < Nirkus> @env['QUERY_STRING'].present? ? @env['QUERY_STRING'] : >> (@env['REQUEST_URI'].split('?', 2)[1] || '') >> >> In CVE-2015-7519[1] it was detected, that it is possible to obtain >> unauthorized access if you send http variables with "_" instead of "-". More >> information can be found here[2]. As a solution it was proposed to simply >> filter out all variables containing an "_". This was already done in mod_cgi >> of apache[3] and now I applied a similar patch to libapache2-mod-passenger >> as well. >> >> Unfortunately there seems to be software that relies on underscores in >> variable names. So if you need such variables you might want to use the >> workaround for apache, described in[2]. > > I am only scratching the surface of Ruby, Passenger, Rack/Rails and > Redminde, so corrections and clarifications welcome. :) > [...] > > I am not sure whether REQUEST_URI and QUERY_STRING are actually passed > as per-request env. variables by Passenger or added to the env hash by > Rack/Rails. > Still, this looks like a regression to me, since it removes previously > available variables, which should not be in scope of CVE-2015-7519. It is a regression, there's no way for applications using mod_passenger to work after the latest update. Not only did the update switch to a native package and drop some documentation, but it broke the module. Granted, the package is safer now that it doesn't work. Regards, -- Raphael Geissert
Re: CVE-2015-7519
Hi, On Tue, Apr 26, 2016 at 10:08 AM, Raphael Geissert wrote: > Hi, > > On 19 February 2016 at 09:35, Linus van Geuns wrote: >> On Thu, Feb 18, 2016 at 8:35 PM, Thorsten Alteholz >> wrote: >>> On irc you wrote: >>> 15:05 < Nirkus> have some old redmine running on squeeze-lts (yeah..) and >>> since the update yesterday the following redmine code bails out with >>> "private method `split' called for nil:NilClass" at the following line: >>> 15:06 < Nirkus> @env['QUERY_STRING'].present? ? @env['QUERY_STRING'] : >>> (@env['REQUEST_URI'].split('?', 2)[1] || '') >>> >>> In CVE-2015-7519[1] it was detected, that it is possible to obtain >>> unauthorized access if you send http variables with "_" instead of "-". >>> More information can be found here[2]. As a solution it was proposed to >>> simply filter out all variables containing an "_". This was already done in >>> mod_cgi of apache[3] and now I applied a similar patch to >>> libapache2-mod-passenger as well. >>> >>> Unfortunately there seems to be software that relies on underscores in >>> variable names. So if you need such variables you might want to use the >>> workaround for apache, described in[2]. >> >> I am only scratching the surface of Ruby, Passenger, Rack/Rails and >> Redminde, so corrections and clarifications welcome. :) >> > [...] >> >> I am not sure whether REQUEST_URI and QUERY_STRING are actually passed >> as per-request env. variables by Passenger or added to the env hash by >> Rack/Rails. >> Still, this looks like a regression to me, since it removes previously >> available variables, which should not be in scope of CVE-2015-7519. > > It is a regression, there's no way for applications using > mod_passenger to work after the latest update. Not only did the update > switch to a native package and drop some documentation, but it broke > the module. > > Granted, the package is safer now that it doesn't work. Yeah, granted "We" are no longer affected by this regression since the affected Redmine instance has been migrated to a current release running on Debian jessie. So, thank you for the incentive to do the right thing. Gruß, Linus
Re: CVE-2015-7519
Hi Thorsten, On Thu, Feb 18, 2016 at 8:35 PM, Thorsten Alteholz wrote: > > [..] > On irc you wrote: > 15:05 < Nirkus> have some old redmine running on squeeze-lts (yeah..) and > since the update yesterday the following redmine code bails out with "private > method `split' called for nil:NilClass" at the following line: > 15:06 < Nirkus> @env['QUERY_STRING'].present? ? @env['QUERY_STRING'] : > (@env['REQUEST_URI'].split('?', 2)[1] || '') > [..] > > In CVE-2015-7519[1] it was detected, that it is possible to obtain > unauthorized access if you send http variables with "_" instead of "-". More > information can be found here[2]. As a solution it was proposed to simply > filter out all variables containing an "_". This was already done in mod_cgi > of apache[3] and now I applied a similar patch to libapache2-mod-passenger as > well. > > Unfortunately there seems to be software that relies on underscores in > variable names. So if you need such variables you might want to use the > workaround for apache, described in[2]. I am only scratching the surface of Ruby, Passenger, Rack/Rails and Redminde, so corrections and clarifications welcome. :) This is my interprtation of the blog entry[1] for CVE-2015-7519: - In order to make HTTP headers of a request available as per-request environment variables, Passenger * prefixes the header names with "HTTP_" * converts those names to upper case * converts all non-alphanumeric characters in header names to underscore ("_") - This behavior allows attackers to pass in per-request env. variables that look like trusted, internal headers to applications (header names "X-User" and "X~User" both get converted to variable name "HTTP_X_USER") Judging from my above interpretation, CVE-2015-7519 should be mitigated by discarding all request headers with names containing other characters than alphanumeric and hyphen ("-"). This is my current understanding of the issue with out legacy Redmine system: - After applying the update of libapache2-mod-passenger to mitigate CVE-2015-7519, libactionpack-ruby1.8 fails to access either of parameters 'QUERY_STRING' and 'REQUEST_URI' - The above-mentioned parameters are retrieved from the Rack/Rails request environment "hash" - The Rails/Rack requests env. "hash"[L1] gets populated based on per-request environment variables received from Passenger - 'QUERY_STRING' and 'REQUEST_URI' are per-request env. variables describing the request passed on py Passenger - Both above-mentioned parameters are not prefixed by "HTTP_" and therefore not in scope of CVE-2015-7519 I am not sure whether REQUEST_URI and QUERY_STRING are actually passed as per-request env. variables by Passenger or added to the env hash by Rack/Rails. Still, this looks like a regression to me, since it removes previously available variables, which should not be in scope of CVE-2015-7519. Gruß, Linus [..] > > [1] https://security-tracker.debian.org/tracker/CVE-2015-7519 > [2] https://blog.phusion.nl/2015/12/07/cve-2015-7519/ > [3] > http://mail-archives.apache.org/mod_mbox/httpd-dev/201010.mbox/<201010121630.19406@apache.org> > [L1]https://quickleft.com/blog/understanding-rack-apps-and-middleware/#the-environment-hash
CVE-2015-7519
Hi Linus, as others might be interested in the answer as well, I also send it to debian-lts@. On irc you wrote: 15:05 < Nirkus> have some old redmine running on squeeze-lts (yeah..) and since the update yesterday the following redmine code bails out with "private method `split' called for nil:NilClass" at the following line: 15:06 < Nirkus> @env['QUERY_STRING'].present? ? @env['QUERY_STRING'] : (@env['REQUEST_URI'].split('?', 2)[1] || '') 15:11 < Nirkus> ah, the code is actually from: libactionpack-ruby1.8: /usr/lib/ruby/1.8/action_controller/request.rb 15:51 < Nirkus> downgrading to libapache2-mod-passenger=2.2.11debian-2 fixes the above issue... In CVE-2015-7519[1] it was detected, that it is possible to obtain unauthorized access if you send http variables with "_" instead of "-". More information can be found here[2]. As a solution it was proposed to simply filter out all variables containing an "_". This was already done in mod_cgi of apache[3] and now I applied a similar patch to libapache2-mod-passenger as well. Unfortunately there seems to be software that relies on underscores in variable names. So if you need such variables you might want to use the workaround for apache, described in[2]. Thorsten [1] https://security-tracker.debian.org/tracker/CVE-2015-7519 [2] https://blog.phusion.nl/2015/12/07/cve-2015-7519/ [3] http://mail-archives.apache.org/mod_mbox/httpd-dev/201010.mbox/<201010121630.19406@apache.org>