
Nginx SPDY heap buffer overflow (affects 1.3.15 – 1.5.11) - jvt
http://nginx.org/en/security_advisories.html?1.5.12
======
thirsteh
Before you panic: "The problem affects nginx 1.3.15 - 1.5.11, compiled with
the ngx_http_spdy_module module (which is not compiled by default) and without
--with-debug configure option, if the "spdy" option of the "listen" directive
is used in a configuration file."

~~~
nevi-me
Well, SPDY is enabled by default on 1.5.11 from what I understand, so they
better push out 1.5.12 out (at least not seeing it yet)

------
AaronFriel
Patch is pretty interesting. Why was buffer overflow protection behind a debug
flag?

[http://nginx.org/download/patch.2014.spdy2.txt](http://nginx.org/download/patch.2014.spdy2.txt)

~~~
unwind
Without context (I'm not familiar with nginx' code) the blatant subtraction of
pointers was scary, too.

As everyone is of course no doubt aware, subtraction of pointers is only valid
in C if you can guarantee that the pointers point to the same "object" (or at
most 1 character past the end of an object).

I would prefer an interface using a size_t count of available space, rather
than passing an end-pointer around.

Again, this is based solely on reading the patch, so it might be perfectly
fine. Considering nginx' reputation, I guess it's academic.

~~~
ambrop7
It's kind of implied by the parameters that they point to the same object;
they're named "pos" and "end" after all. But I agree, a count variable is
nicer.

The subtraction also has a (very theoretical) problem that its result must fit
in ptrdiff_t (signed), and it would produce undefined behavior when you have a
huge object whose size doesn't fit in ptrdiff_t (but does in size_t, by
definition).

------
IvyMike

      -        if (ngx_list_init(&r->headers_in.headers, r->pool, sc->entries + 3,
      +        if (ngx_list_init(&r->headers_in.headers, r->pool, 20,
    

Personally, both the old code and the new code use magic numbers, and both
send up a red flag to me. What does 20 mean? (I don't personally need an
answer here: my point is it's not obvious and therefore it's easier for bugs
to get through.)

------
jmnicolas
I wonder if this discovery is a result of OpenBSD switching its focus from
Apache to Nginx ?

~~~
jolan
It wasn't.

> Thanks to Lucas Molas, researcher at Programa STIC, Fundación Dr. Manuel
> Sadosky, Buenos Aires, Argentina.

------
dperfect
I assume this only affects configurations with ngx_http_spdy_module enabled.
Can anyone confirm whether or not that's the case?

~~~
fletchowns
Yup, it's in the announcement: [http://mailman.nginx.org/pipermail/nginx-
announce/2014/00013...](http://mailman.nginx.org/pipermail/nginx-
announce/2014/000135.html)

 _The problem affects nginx 1.3.15 - 1.5.11, compiled with the
ngx_http_spdy_module module (which is not compiled by default) and without
--with-debug configure option, if the "spdy" option of the "listen" directive
is used in a configuration file._

------
yeukhon
It said it affected 1.3.15 to 1.5.11 but the fix is available _in nginx
1.5.12, 1.4.7_.

So for those who are using legacy version are they going to rely on distro
vendor to push the patch? Just curious, even though I guess the number of
users who have activated this experimental SPDY is low and people who actually
have it enabled probably know how to fix it themselves.

~~~
aroch
1.5.12 is the latest devel branch and 1.4.7 is the latest release branch
(released in response to this CVE and excepted from the vuln range). The patch
works for any of the intervening versions.

As far as I'm aware, all the major distros backport nginx stable release

------
jxf
Why was it replaced with "#if 1" instead of just deleting the entire "if/end"
delimiters? Won't that always evaluate to true?

~~~
mherkender
Maybe they wanted a patch that could be easily applied to older versions. It
might have an elif later too, although I'm guessing that's not too common
after debug blocks.

------
ryanseys
It appears my Ubuntu server only updates nginx mainline to 1.5.11. I've
disabled spdy support in my listen directive for now as an easier work around.

~~~
atom7
It's not affected on Debian/Ubuntu, check --with-debug flag.

~~~
ryanseys
Thank you.

~~~
atom7
btw, I use packages from nginx.org, they're updated in time with every
release:
[http://nginx.org/en/linux_packages.html](http://nginx.org/en/linux_packages.html)

~~~
voltagex_
Do the upstream packages still use a different configuration path from the
Debian/Ubuntu packages?

~~~
kijin
Yes. There's no sites-available or sites-enabled, and some configuration is
split off into different files in the conf.d directory.

------
pjmlp
Got to love C.

~~~
Fasebook
Almost as much as I love Java

