overlapping memcpy in ssl_io_input_read

[Jiří Engelthaler]

Hello.
I found a terrible bug in memory copy routine (eglibc-2.11.1/sysdeps/i386/i686/multiarch/memcpy-ssse3-rep.S and memcpy-ssse3.S). Here is the code

    movl LEN(%esp), %ecx
    movl SRC(%esp), %eax
    movl DEST(%esp), %edx
......
L(fwd_write_less32bytes):
#ifndef USE_AS_MEMMOVE
    cmp %dl, %al -----<<<<< BUG !!!
    jb L(bk_write)

Assume, that I have an array of char AR and I want move data from AR[10] to AR[0] with length 47. Data should be copied in forward direction. Problem is when AR[10] overlaps 255 bytes boundary. For example address of AR[0] is 0x000000F8 (EDX) and address of AR[10] is 0x00000102 (EAX) then cmp %dl,%al return AL as smaller then DL and the data are copied in reverse direction that causes data corruption.
In reality it will cause problems on Ubuntu 10.04 mod_ssl in Apache with last updates installed (libc6: Installed: 2.11.1-0ubuntu7.2), which are sometimes unable to process the HTTP header because of malformed data, but may cause other unexpected behavior (bug #595116, bug #595855, bug #589611 and maybe others).

I don't know if only this two files is affected by this bug.

See GDB snapshot in attachment

  Regards
     Jiri Engelthaler

[Jiří Engelthaler (engycz)]

This test code should return always the same string but it doesn't.

[Stefan Fritsch (sf-sfritsch)]

Thanks for digging into this, but I thing your analysis is wrong. From "man 3posix memcpy":

"The memcpy() function shall copy n bytes from the object pointed to by s2 into the object
pointed to by s1. If copying takes place between objects that overlap, the behavior is undefined."

mod_ssl must use memmove if the buffers may overlap. Of course, the overlapping buffers may be another bug in mod_ssl. I will look into it.

[Stefan Fritsch (sf-sfritsch)]

This is https://issues.apache.org/bugzilla/show_bug.cgi?id=45444

Patch is at http://svn.apache.org/viewvc?rev=683280&view=rev

[Jiří Engelthaler (engycz)]

Yes you are right. The bug was fixed in main trunk of apache but not in 2.2.x branch.
memcpy-ssse3 has code for forward and reverse copy. Why?

However, I suggest you add a debug statement to memcpy, to monitor overlapping calls, whether it's just the isolated case.

Maybe some developers quietly ignore the note about overlapping in memcpy and then they are surprises.

[Launchpad Janitor (janitor)]

This bug was fixed in the package apache2 - 2.2.16-1ubuntu1

---------------
apache2 (2.2.16-1ubuntu1) maverick; urgency=low

  * Merge from debian unstable. Remaining changes:
    - debian/{control, rules}: Enable PIE hardening.
    - debian/{control, rules, apache2.2-common.ufw.profile}: Add ufw profiles.
    - debian/control: Add bzr tag and point it to our tree.
    - debian/apache2-2.common.apache2.init: Add graceful restart (LP: #456381)

apache2 (2.2.16-1) unstable; urgency=medium

  * Urgency medium for security fix.
  * New upstream release:
    - CVE-2010-1452: mod_dav, mod_cache: Fix denial of service vulnerability
      due to incorrect handling of requests without a path segment.
    - mod_dir: add FallbackResource directive, to enable admin to specify
      an action to happen when a URL maps to no file, without resorting
      to ErrorDocument or mod_rewrite
  * Fix mod_ssl header line corruption because of using memcpy for overlapping
    buffers. PR 45444. LP: #609290, #589611, #595116

apache2 (2.2.15-6) unstable; urgency=low

  * Fix init script not correctly killing htcacheclean. Closes: #580971
  * Add a separate entry in README.Debian about the need to use apache2ctl
    for starting instead of calling apache2 directly. Closes: #580445
  * Fix debug info to allow gdb loading it automatically. Closes: #581514
  * Fix install target in Makefile created by apxs2 -n. Closes: #588787
  * Fix ab sending more requests than specified by the -n parameter.
    Closes: #541158
  * Add apache2 monit configuration to apache2.2-commons examples dir.
    Closes: #583127
  * Build as PIE, since gdb in squeeze now supports it.
  * Update the postrm script to also purge the version of /var/www/index.html
    introduced in 2.2.11-7.
  * Bump Standards-Version (no changes).
 -- Chuck Short <email address hidden> Mon, 26 Jul 2010 20:21:37 +0100

[Jiří Engelthaler (engycz)]

I hope this will be fixed as soon as possible. It's very simple patch.

[Jonathan Riddell (jr)]

Bug should have a debdiff and test case included

Waiting in lucid-proposed unapproved queue for ubuntu-sru approval

[Jiří Engelthaler (engycz)]

It's my first debdiff, so I don't know if it is correct.

[John Dong (jdong)]

ACK from SRU team, but I'd like a test case to be added for the meantime. The code for the test case is there, but the procedure is not.

[Lars Hvile (lars-fovea)]

Is there any news on this issue? We're waiting anxiously for a fix =) What kind of test case should be added, wasn't this a bug which was fixed in apache-trunk over two years ago?

[Martin Pitt (pitti)]

Matthias, I reject your upload and take Chuck's, which also refers to two other bugs.

[Martin Pitt (pitti)]

Accepted apache2 into lucid-proposed, the package will build now and be available in a few hours. Please test and give feedback here. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Thank you in advance!

[Lars Hvile (lars-fovea)]

has anyone been able to build the new apache2 from lucid-proposed? I'm getting an error while building...

$apt-get -b source apache2
......
applying patch 206-report-max-client-mpm-worker to ./ ... ok.
applying patch 209-backport-mod-reqtimeout to ./ ... ok.
applying patch 210-backport-mod-reqtimeout-ftbfs to ./ ... ok.
applying patch upstream-fix-for-lp-609290.patch to ./ ...diff: httpd-2.2.14.orig//modules/ssl/ssl_engine_io.c: No such file or directory
diff: httpd-2.2.14//modules/ssl/ssl_engine_io.c: No such file or directory
/home/administrator/apache-ssl-fix/apache2-2.2.14/debian/patches/upstream-fix-for-lp-609290.patch: line 2: ---: command not found
/home/administrator/apache-ssl-fix/apache2-2.2.14/debian/patches/upstream-fix-for-lp-609290.patch: line 3: +++: command not found
/home/administrator/apache-ssl-fix/apache2-2.2.14/debian/patches/upstream-fix-for-lp-609290.patch: line 4: @@: command not found
/home/administrator/apache-ssl-fix/apache2-2.2.14/debian/patches/upstream-fix-for-lp-609290.patch: line 5: ABOUT_APACHE: command not found
/home/administrator/apache-ssl-fix/apache2-2.2.14/debian/patches/upstream-fix-for-lp-609290.patch: line 21: syntax error near unexpected token `('
/home/administrator/apache-ssl-fix/apache2-2.2.14/debian/patches/upstream-fix-for-lp-609290.patch: line 21: `- memcpy(in, buffer->value, inl);'
 failed.
make: *** [patch-stamp] Error 1
dpkg-buildpackage: error: debian/rules build gave error exit status 2
Build command 'cd apache2-2.2.14 && dpkg-buildpackage -b -uc' failed.

[Steve Langasek (vorlon)]

No, this package has failed to build on all architectures. I've pulled the package back out of lucid-proposed. Chuck, please build test packages before uploading them as SRUs.

[Lars Hvile (lars-fovea)]

still no news on this issue? I've installed the attempted fix 2.2.14-5ubuntu8.2, from #595116.. But I'm still getting the same error.

[Jiří Engelthaler (engycz)]

2.2.14-5ubuntu8.2 doesn't contain fix for this bug.

[Ingo Rohlfs (ingo-rohlfs)]

Can someone please release a fix for this. I have several servers here facing this error.

[Jiří Engelthaler (engycz)]

Hello.
  Because ubuntu team is not able to release the bugfix over 1 month, I compiled patched mod_ssl. For those who want to try it, it's at http://engy.dyndns.org/mod_ssl.so

[Lars Hvile (lars-fovea)]

Jiří: What happened with the patch you posted 2010-08-04? Was it rejected?

[Andre van der Elst (andre-finalist)]

Jiri,

I'm trying it out and will watch my logfiles for a few days.

[Jiří Engelthaler (engycz)]

They used patch from https://bugs.launchpad.net/ubuntu/+source/apache2/+bug/589611 comment #35, but it contains error - comment #14 from this bug.

[Lars Hvile (lars-fovea)]

Jiří: your binary seems do to the trick, I haven't seen the error anymore since updating to it, so thanks, even though I don't really feel comfortable using an unofficial binary fix for this..

[Jiří Engelthaler (engycz)]

Thx. Fixing this bug is next in queue. See https://bugs.launchpad.net/ubuntu/lucid/+source/apache2/+bug/589611

[Martin Pitt (pitti)]

Accepted apache2 into lucid-proposed, the package will build now and be available in a few hours. Please test and give feedback here. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Thank you in advance!

[Launchpad Janitor (janitor)]

This bug was fixed in the package apache2 - 2.2.14-5ubuntu8.3

---------------
apache2 (2.2.14-5ubuntu8.3) lucid-proposed; urgency=low

  * debian/apache2.2-common.postinst: Don't fail if you can load the reqtimeout module.
    (LP: #621837)
  * debian/patches/Backport fix for upstream bug PR 45444: https://issues.apache.org/bugzilla/show_bug.cgi?id=45444. (LP: #609290, #589611, #595116)
 -- Chuck Short <email address hidden> Mon, 27 Sep 2010 14:06:57 -0400

[Jiří Engelthaler (engycz)]

Great. It's a very quick bugfix for this high importance bug - after 3 months from the known solutions.