Skip to content

Commit

Permalink
Fix CVE-2015-7519 header collision vulnerability
Browse files Browse the repository at this point in the history
  • Loading branch information
FooBarWidget committed Dec 7, 2015
1 parent 988af70 commit ddb8ecc
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 6 deletions.
1 change: 1 addition & 0 deletions CHANGELOG
Original file line number Diff line number Diff line change
@@ -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.

This comment has been minimized.

Copy link
@runephilosof

runephilosof Jan 26, 2016

This needs to be clarified more.
It is backwards incompatible change that will break apps using request headers that contain underscores.
Warn about it and link to the workaround blog post https://blog.phusion.nl/2015/12/07/cve-2015-7519/

* [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.
Expand Down
45 changes: 39 additions & 6 deletions src/agent/Core/Controller/SendRequest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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] = {
Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit ddb8ecc

Please sign in to comment.