Re: diff for passenger in Squeeze

[Thorsten Alteholz]

Hi Guido and Antoine,

thanks alot for looking at the patch, yes I forgot an exclamation mark.

The patch has not been tested yet (this is next on my agenda). I only 
wanted to make sure that I am on the right track. Indeed, between those 
versions there has been a major rewrite and the version in Squeeze didn't 
use the LString yet.


  Thorsten

Re: diff for passenger in Squeeze

[Antoine Beaupré]

On 2015-12-29 07:36:30, Guido Günther wrote:
> Hi Thorsten,
>
> Isn't the logic reversed here? We want so _skip_ the header if it
> containsNonAlphaNumDash not add it?

After reviewing the patch, I agree, the logic is reversed.

I am also not sure why the patch is so different - did the primitives
change so much between the two versions that there's a new string type
and all?

Was the patch tested?

A.

-- 
A ballot is like a bullet. You don't throw your ballots until you see
a target, and if that target is not within your reach, keep your
ballot in your pocket.
 - Malcom X

Re: diff for passenger in Squeeze

[Guido Günther]

Hi Thorsten,
On Mon, Dec 28, 2015 at 11:13:32PM +0100, Thorsten Alteholz wrote:
> Hi everybody,
> 
> can someone please have a look at the diff for passenger=2.2.11debian-2 in
> Squeeze that should solve CVE-2015-7519[1] and nod?
> 
> Thanks!
>  Thorsten
> 
> 
> [1] https://security-tracker.debian.org/tracker/CVE-2015-7519
> 
> 
> 
> diff -Nru passenger-2.2.11debian/ext/apache2/Hooks.cpp 
> passenger-2.2.11debian/ext/apache2/Hooks.cpp
> --- passenger-2.2.11debian/ext/apache2/Hooks.cpp2010-03-05 
> 10:35:16.0 +0100
> +++ passenger-2.2.11debian/ext/apache2/Hooks.cpp2015-12-28 
> 20:04:14.0 +0100
> @@ -779,9 +779,33 @@
> 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) {
> +   if ((name != NULL && value != NULL) || 
> containsNonAlphaNumDash(name)) {
> apr_table_addn(table, name, value);

Isn't the logic reversed here? We want so _skip_ the header if it
containsNonAlphaNumDash not add it?
Cheers,
 -- Guido