8 ms·

Security vulnerability found in Nginx

170 points by dirtyaura 15y ago

matthavener 15y ago
I may be misinterpreting the patch, but it seems like this is a NULL byte injection vulnerability, all stemming from '\0' termination in C-strings. I wonder if length-buffer pairs are more practical when security is a consideration? Similar issues in Perl: http://www.phrack.org/issues.html?issue=55&id=7#article http://www.phrack.org/issues.html?issue=55&id=7#article and other languages: http://hakipedia.com/index.php/Poison_Null_Byte#Perl_PHP_Null_Byte_Injection http://hakipedia.com/index.php/Poison_Null_Byte#Perl_PHP_Nul...
- gizzlon 15y ago
  Not a "C guy", but I simply read it as: Strings could be missing a "\0" and therefore some function could read too far later on..
  - matthavener 15y ago
    Not exactly. Some function expecting a c-string could actually read too short. What happens is some attacker sends a message using a channel/format that specifies a length and a buffer. But, the buffer contains a "\0". For example, (10, "wtf\0extra") is passed as a 10 char string. But, some c function might only see it as (3,"wtf"). The (usually) simply solution is to fail validation when a string of N bytes contains a NULL char before the N+1 byte.
- wladimir 15y ago
  IMO, zero-terminated strings are, and always have been, a bad idea. Discriminating against a certain character instead of just regarding it as opaque blob of data will bite you in unexpected ways, and strlen() taking O(N) is another drawback. The string functions in the C standard library suck. bstring is a better alternative: http://bstring.cvs.sourceforge.net/viewvc/bstring/tree/bstrlib.txt?pathrev=HEAD http://bstring.cvs.sourceforge.net/viewvc/bstring/tree/bstrl... . Glib also has a good string library...
  - kbolino 15y ago
    Something to bear in mind regarding non-standard string libraries: syscalls (like open[1] on POSIX or CreateFile[2] on Windows) use null-terminated strings, which means that you have to be careful about embedded nulls, no matter what library you use, when a string gets passed to a syscall somewhere in the chain. [1] http://pubs.opengroup.org/onlinepubs/000095399/functions/open.html http://pubs.opengroup.org/onlinepubs/000095399/functions/ope... [2] http://msdn.microsoft.com/en-us/library/windows/desktop/aa363858%28v=vs.85%29.aspx http://msdn.microsoft.com/en-us/library/windows/desktop/aa36...
    - wladimir 15y ago
      Yes. You are right. In general, you have to be really careful when passing user-provided input to syscalls. Embedded nulls are only one of the many pitfalls there (another one from the top of my head is unicode handling). Especially if you're writing a webserver.
    - barrkel 15y ago
      It may be interesting to note that Windows syscalls (i.e. to the NT kernel rather than the Win32 layer wrappers like CreateFile) do not actually use null-terminated strings - they use UNICODE_STRING[1], which is a structure containing a 16-bit length, 16-bit buffer length, and pointer to a buffer of 2-byte characters. NtCreateFile[2] (and the kernel-side implementation of ZwCreateFile[3]) take a file name in the form of OBJECT_ATTRIBUTES, whose ObjectName field is of type PUNICODE_STRING. CreateFile is implemented in terms of NtCreateFile; CreateFile enforces Win32 semantics like case insensitivity that NtCreateFile does not; POSIX semantics can be implemented on top of NtCreateFile, but not easily with CreateFile. [1] http://msdn.microsoft.com/en-us/library/windows/hardware/ff564879%28v=vs.85%29.aspx http://msdn.microsoft.com/en-us/library/windows/hardware/ff5... [2] http://msdn.microsoft.com/en-us/library/bb432380%28v=vs.85%29.aspx http://msdn.microsoft.com/en-us/library/bb432380%28v=vs.85%2... [3] http://msdn.microsoft.com/en-us/library/windows/hardware/ff566424%28v=vs.85%29.aspx http://msdn.microsoft.com/en-us/library/windows/hardware/ff5...
      - deleted 15y ago
        [deleted]
jaryd 15y ago
Hey guys, Looking for a consensus on the most stable way to update nginx installations from source. Thanks!
- krobertson 15y ago
  I prefer to custom build deb packages (on Ubuntu myself) using a project called brew2deb: https://github.com/tmm1/brew2deb https://github.com/tmm1/brew2deb It provides a DSL for describing how to build and package something, including patching, and already has a nginx formula: https://github.com/tmm1/brew2deb/blob/master/packages/nginx/formula.rb https://github.com/tmm1/brew2deb/blob/master/packages/nginx/... Will likely be adding in this patch today...
  - pja 15y ago
    It's a relatively simple task if you're on a Debian derivative to take the source .deb & add a new patch to it.
  - ef4 15y ago
    I second the idea of building your own deb. But if you're going to learn a DSL for building debs, why not just learn how to build debs? Your distro already provides the necessary build formula, along with well-integrated startup scripts, etc. To start out, just build as given: apt-get build-dep nginx apt-get source nginx cd nginx-$VERSION dpkg-buildpackage Then to customize, go into the source tree and update whatever you want to update, and rebuild. You'll see there's already a debian/patches directory where you can drop patches to apply automatically. I tend to just keep the "debian" directory in source control, so I can take a fresh upstream tarball, check out my debian rules into it, and kick off the build.
    - krobertson 15y ago
      I've done custom debian packages from scratch and always found it a pain. I've also done customized source builds and still prefer this approach because its simple, slimmer, easily version controlled, and wrapped up in a single command. But there are many ways to fry an egg. I prefer this way, but overall having a package is a big plus for commonality between systems and convenience vs simply building from source.
digitalsushi 15y ago
The ides of march bug.
tptacek 15y ago
This is a very bad bug, and you should fix it ASAP. Don't wait.
- some32 15y ago
  It is already fixed.
- spindritf 15y ago
  What are the possible implications?
  - tptacek 15y ago
    You can safely trust me on this. Also, if you're concerned, the patch is very straightforward, minimal, safe, and fairly unintrusive. It's not going to break anything. Just patch it.
    - spindritf 15y ago
      I did, I never pretend to know better than the developers. Question was dictated more by curiosity.
      - tptacek 15y ago
        I understand and totally don't blame you for asking.
        coderdude 15y ago
        This is all very dramatic and entertaining, and we all know you're the security expert, so why leave that comment here yet decline to elaborate?
        tptacek 15y ago
        Don't you think there should be a term for the phenomenon where someone makes it more or less clear that they have said all they're going to say about something on a message board, and still people come out of the woodwork to write comments cajoling them into saying more? I'm not complaining. It doesn't happen to me that often. Way more often, it's someone complaining about some anonymous employer or service provider and 20 people writing comments about how it's irresponsible for them not to say who it was. But it's the same kind of annoying every time. Maybe the term ought to be in German. German works great for concepts like this. Why did I leave the comment? Because it's hard to patch server software and people will often wait on patches until maintenance windows (advise you not do that this time) or take some time to figure out if they're affected. Especially with Apache, where oftentimes you aren't affected because the bug is in some random module most people don't use.
uggedal 15y ago
Arch Linux already has the 1.0.14 release available in the community repo[1]. There are still no new patch level version of 0.7.67 available for Debian Squeeze[2] or Ubutu 10.4 LTS' 0.7.65 version[3]. EPEL for RHEL and derivatives also lack a new upstream version[4]. [1]: http://projects.archlinux.org/svntogit/community.git/commit/trunk?h=packages/nginx&id=9fa938f3189c65b4d4e30d6f82868f59b8dce503 http://projects.archlinux.org/svntogit/community.git/commit/... [2]: http://packages.debian.org/changelogs/pool/main/n/nginx/?C=M;O=D http://packages.debian.org/changelogs/pool/main/n/nginx/?C=M... [3]: http://changelogs.ubuntu.com/changelogs/pool/universe/n/nginx/?C=M;O=D http://changelogs.ubuntu.com/changelogs/pool/universe/n/ngin... [4]: http://dl.fedoraproject.org/pub/epel/6/x86_64/repoview/nginx.html http://dl.fedoraproject.org/pub/epel/6/x86_64/repoview/nginx...
- IgorPartola 15y ago
  Re [Debian/Ubuntu], wait, they incorporated the fix on 17 June 2011, but the patch is being exposed now on the nginx mailing list? Edit: changed [2] to [Debian/Ubuntu]
  - deleted 15y ago
    [deleted]
  - uggedal 15y ago
    The vulnerability from last year sounded similar and I therefore initialy mixed them. Updated my comment.
- Wilya 15y ago
  1.0.14[0] and 1.1.17[1] have been pushed to the FreeBSD ports tree too. [0] http://www.freebsd.org/cgi/getmsg.cgi?fetch=753841+0+current/cvs-ports http://www.freebsd.org/cgi/getmsg.cgi?fetch=753841+0+current... [1] http://www.freebsd.org/cgi/getmsg.cgi?fetch=756257+0+current/cvs-ports http://www.freebsd.org/cgi/getmsg.cgi?fetch=756257+0+current...
- wkral 15y ago
  1.0.14 has also been released in the Ubuntu PPA for those who use Ubuntu: https://launchpad.net/~nginx/+archive/stable https://launchpad.net/~nginx/+archive/stable 1.1.17 is also there in the development channel: https://launchpad.net/~nginx/+archive/development https://launchpad.net/~nginx/+archive/development
rdl 15y ago
This is particularly interesting because in a lot of deployments, nginx sits out in front of a lot of other stuff as a load balancer, where it is nicely exposed. You REALLY should be using multiple boxes if you're running load balancers (especially sw load balancers) with some kind of heartbeat failover. That way you can upgrade single boxes easily, and are ok in case one of them dies. With a bug of this severity, you won't have time to test the patch, so it's probably best to upgrade one at a time in production. Remember, even if you're running Apache or something else for your actual web server, you can easily have something like nginx sitting in front as a proxy/load balancer. Often in front of your security monitoring devices... and you may have forgotten about it.
aaronblohowiak 15y ago
This only matters if your backend is going to set a header that contains a null byte. Since some people echo back user data in headers (ugh) this could cause an issue. Rails is more than happy to let you put NULLs in response headers, btw. Of course, all of the ASCII CTL characters (0..27) are forbidden by the spec: http://www.ietf.org/rfc/rfc2068.txt http://www.ietf.org/rfc/rfc2068.txt

170 points by dirtyaura 15y ago

I may be misinterpreting the patch, but it seems like this is a NULL byte injection vulnerability, all stemming from '\0' termination in C-strings. I wonder if length-buffer pairs are more practical when security is a consideration? Similar issues in Perl: http://www.phrack.org/issues.html?issue=55&id=7#article http://www.phrack.org/issues.html?issue=55&id=7#article and other languages: http://hakipedia.com/index.php/Poison_Null_Byte#Perl_PHP_Null_Byte_Injection http://hakipedia.com/index.php/Poison_Null_Byte#Perl_PHP_Nul...

gizzlon 15y ago
Not a "C guy", but I simply read it as: Strings could be missing a "\0" and therefore some function could read too far later on..
- matthavener 15y ago
  Not exactly. Some function expecting a c-string could actually read too short. What happens is some attacker sends a message using a channel/format that specifies a length and a buffer. But, the buffer contains a "\0". For example, (10, "wtf\0extra") is passed as a 10 char string. But, some c function might only see it as (3,"wtf"). The (usually) simply solution is to fail validation when a string of N bytes contains a NULL char before the N+1 byte.
wladimir 15y ago
IMO, zero-terminated strings are, and always have been, a bad idea. Discriminating against a certain character instead of just regarding it as opaque blob of data will bite you in unexpected ways, and strlen() taking O(N) is another drawback. The string functions in the C standard library suck. bstring is a better alternative: http://bstring.cvs.sourceforge.net/viewvc/bstring/tree/bstrlib.txt?pathrev=HEAD http://bstring.cvs.sourceforge.net/viewvc/bstring/tree/bstrl... . Glib also has a good string library...
- kbolino 15y ago
  Something to bear in mind regarding non-standard string libraries: syscalls (like open[1] on POSIX or CreateFile[2] on Windows) use null-terminated strings, which means that you have to be careful about embedded nulls, no matter what library you use, when a string gets passed to a syscall somewhere in the chain. [1] http://pubs.opengroup.org/onlinepubs/000095399/functions/open.html http://pubs.opengroup.org/onlinepubs/000095399/functions/ope... [2] http://msdn.microsoft.com/en-us/library/windows/desktop/aa363858%28v=vs.85%29.aspx http://msdn.microsoft.com/en-us/library/windows/desktop/aa36...
  - wladimir 15y ago
    Yes. You are right. In general, you have to be really careful when passing user-provided input to syscalls. Embedded nulls are only one of the many pitfalls there (another one from the top of my head is unicode handling). Especially if you're writing a webserver.
  - barrkel 15y ago
    It may be interesting to note that Windows syscalls (i.e. to the NT kernel rather than the Win32 layer wrappers like CreateFile) do not actually use null-terminated strings - they use UNICODE_STRING[1], which is a structure containing a 16-bit length, 16-bit buffer length, and pointer to a buffer of 2-byte characters. NtCreateFile[2] (and the kernel-side implementation of ZwCreateFile[3]) take a file name in the form of OBJECT_ATTRIBUTES, whose ObjectName field is of type PUNICODE_STRING. CreateFile is implemented in terms of NtCreateFile; CreateFile enforces Win32 semantics like case insensitivity that NtCreateFile does not; POSIX semantics can be implemented on top of NtCreateFile, but not easily with CreateFile. [1] http://msdn.microsoft.com/en-us/library/windows/hardware/ff564879%28v=vs.85%29.aspx http://msdn.microsoft.com/en-us/library/windows/hardware/ff5... [2] http://msdn.microsoft.com/en-us/library/bb432380%28v=vs.85%29.aspx http://msdn.microsoft.com/en-us/library/bb432380%28v=vs.85%2... [3] http://msdn.microsoft.com/en-us/library/windows/hardware/ff566424%28v=vs.85%29.aspx http://msdn.microsoft.com/en-us/library/windows/hardware/ff5...
    - deleted 15y ago
      [deleted]

jaryd 15y ago

Hey guys, Looking for a consensus on the most stable way to update nginx installations from source. Thanks!

krobertson 15y ago
I prefer to custom build deb packages (on Ubuntu myself) using a project called brew2deb: https://github.com/tmm1/brew2deb https://github.com/tmm1/brew2deb It provides a DSL for describing how to build and package something, including patching, and already has a nginx formula: https://github.com/tmm1/brew2deb/blob/master/packages/nginx/formula.rb https://github.com/tmm1/brew2deb/blob/master/packages/nginx/... Will likely be adding in this patch today...
- pja 15y ago
  It's a relatively simple task if you're on a Debian derivative to take the source .deb & add a new patch to it.
- ef4 15y ago
  I second the idea of building your own deb. But if you're going to learn a DSL for building debs, why not just learn how to build debs? Your distro already provides the necessary build formula, along with well-integrated startup scripts, etc. To start out, just build as given: apt-get build-dep nginx apt-get source nginx cd nginx-$VERSION dpkg-buildpackage Then to customize, go into the source tree and update whatever you want to update, and rebuild. You'll see there's already a debian/patches directory where you can drop patches to apply automatically. I tend to just keep the "debian" directory in source control, so I can take a fresh upstream tarball, check out my debian rules into it, and kick off the build.
  - krobertson 15y ago
    I've done custom debian packages from scratch and always found it a pain. I've also done customized source builds and still prefer this approach because its simple, slimmer, easily version controlled, and wrapped up in a single command. But there are many ways to fry an egg. I prefer this way, but overall having a package is a big plus for commonality between systems and convenience vs simply building from source.

digitalsushi 15y ago

The ides of march bug.

tptacek 15y ago

This is a very bad bug, and you should fix it ASAP. Don't wait.

some32 15y ago
It is already fixed.
spindritf 15y ago
What are the possible implications?
- tptacek 15y ago
  You can safely trust me on this. Also, if you're concerned, the patch is very straightforward, minimal, safe, and fairly unintrusive. It's not going to break anything. Just patch it.
  - spindritf 15y ago
    I did, I never pretend to know better than the developers. Question was dictated more by curiosity.
    - tptacek 15y ago
      I understand and totally don't blame you for asking.
      - coderdude 15y ago
        This is all very dramatic and entertaining, and we all know you're the security expert, so why leave that comment here yet decline to elaborate?
        tptacek 15y ago
        Don't you think there should be a term for the phenomenon where someone makes it more or less clear that they have said all they're going to say about something on a message board, and still people come out of the woodwork to write comments cajoling them into saying more? I'm not complaining. It doesn't happen to me that often. Way more often, it's someone complaining about some anonymous employer or service provider and 20 people writing comments about how it's irresponsible for them not to say who it was. But it's the same kind of annoying every time. Maybe the term ought to be in German. German works great for concepts like this. Why did I leave the comment? Because it's hard to patch server software and people will often wait on patches until maintenance windows (advise you not do that this time) or take some time to figure out if they're affected. Especially with Apache, where oftentimes you aren't affected because the bug is in some random module most people don't use.

uggedal 15y ago

Arch Linux already has the 1.0.14 release available in the community repo[1]. There are still no new patch level version of 0.7.67 available for Debian Squeeze[2] or Ubutu 10.4 LTS' 0.7.65 version[3]. EPEL for RHEL and derivatives also lack a new upstream version[4]. [1]: http://projects.archlinux.org/svntogit/community.git/commit/trunk?h=packages/nginx&id=9fa938f3189c65b4d4e30d6f82868f59b8dce503 http://projects.archlinux.org/svntogit/community.git/commit/... [2]: http://packages.debian.org/changelogs/pool/main/n/nginx/?C=M;O=D http://packages.debian.org/changelogs/pool/main/n/nginx/?C=M... [3]: http://changelogs.ubuntu.com/changelogs/pool/universe/n/nginx/?C=M;O=D http://changelogs.ubuntu.com/changelogs/pool/universe/n/ngin... [4]: http://dl.fedoraproject.org/pub/epel/6/x86_64/repoview/nginx.html http://dl.fedoraproject.org/pub/epel/6/x86_64/repoview/nginx...

IgorPartola 15y ago
Re [Debian/Ubuntu], wait, they incorporated the fix on 17 June 2011, but the patch is being exposed now on the nginx mailing list? Edit: changed [2] to [Debian/Ubuntu]
- deleted 15y ago
  [deleted]
- uggedal 15y ago
  The vulnerability from last year sounded similar and I therefore initialy mixed them. Updated my comment.
Wilya 15y ago
1.0.14[0] and 1.1.17[1] have been pushed to the FreeBSD ports tree too. [0] http://www.freebsd.org/cgi/getmsg.cgi?fetch=753841+0+current/cvs-ports http://www.freebsd.org/cgi/getmsg.cgi?fetch=753841+0+current... [1] http://www.freebsd.org/cgi/getmsg.cgi?fetch=756257+0+current/cvs-ports http://www.freebsd.org/cgi/getmsg.cgi?fetch=756257+0+current...
wkral 15y ago
1.0.14 has also been released in the Ubuntu PPA for those who use Ubuntu: https://launchpad.net/~nginx/+archive/stable https://launchpad.net/~nginx/+archive/stable 1.1.17 is also there in the development channel: https://launchpad.net/~nginx/+archive/development https://launchpad.net/~nginx/+archive/development

rdl 15y ago

This is particularly interesting because in a lot of deployments, nginx sits out in front of a lot of other stuff as a load balancer, where it is nicely exposed. You REALLY should be using multiple boxes if you're running load balancers (especially sw load balancers) with some kind of heartbeat failover. That way you can upgrade single boxes easily, and are ok in case one of them dies. With a bug of this severity, you won't have time to test the patch, so it's probably best to upgrade one at a time in production. Remember, even if you're running Apache or something else for your actual web server, you can easily have something like nginx sitting in front as a proxy/load balancer. Often in front of your security monitoring devices... and you may have forgotten about it.

aaronblohowiak 15y ago

This only matters if your backend is going to set a header that contains a null byte. Since some people echo back user data in headers (ugh) this could cause an issue. Rails is more than happy to let you put NULLs in response headers, btw. Of course, all of the ASCII CTL characters (0..27) are forbidden by the spec: http://www.ietf.org/rfc/rfc2068.txt http://www.ietf.org/rfc/rfc2068.txt