Bug 1095602

Description of problem:

The patch file tomcat6-6.0.24-CVE-2013-4322.patch which is in tomcat6-6.0.24-64.el6_5.src.rpm has a typo in code copied from tomcat7 which crashes my web application which uses chunked transfer. The code in tomcat7 has a comparison testing for less than zero, and the patch file incorrectly tests for greater than 0. Details are below.

Version-Release number of selected component (if applicable):

  tomcat6-6.0.24-64.el6_5.src.rpm

How reproducible:

  My application always fails after installing this rpm, works after a yum downgrade, fails again if I upgrade again. If I disable chunked transfer in the client then it does not get the exception. I did not see the failure in a small test case that uses chunked transfer. However, the bug is obvious if you look at the compare the line of code I will describe in the patch file with the corresponding code in the original tomcat 7 source it is copied from.

  The effect of the typo is that when trying to look for extension or transfer headers at the end of chunked data, if an internal read buffer has been completely read and more data needs to be read in, and this happens at a particular path through the code, a check there for end of the stream throws the EOFException when there is more data available instead of when there is not more data available.

Steps to Reproduce:

  Inside tomcat6-6.0.24-64.el6_5.src.rpm look at file
tomcat6-6.0.24-CVE-2013-4322.patch
and locate the lines

-        return parseCRLF(); // FIXME
+    private boolean parseHeader() 
+        throws IOException {
+        
+        MimeHeaders headers = request.getMimeHeaders();
+        byte chr = 0;
 
+        // Read new bytes if needed
+        if (pos >= lastValid) {
+            if (readBytes() > 0)
+                throw new EOFException(
+                    "Unexpected end of stream while reading trailer headers for chunked request");
+        }
+

The method readBytes() indicates an EOF by returning a negative number, so the if test is backwards in the two lines

+        if (pos >= lastValid) {
+            if (readBytes() > 0)

Notice that following it in the same chunk of the patch file there are four more instances of this, except with the test being for less than 0 (as it should be)

+        if (pos >= lastValid) {
+            if (readBytes() < 0)


Now look at the original code by searching for

private boolean parseHeader()

in the tomcat 7 version of this source code on this page

http://svn.apache.org/repos/asf/tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/filters/ChunkedInputFilter.java


Actual results:

The Apache Tomcat 7 version of the code says if (readBytes() < 0)
The patch file containing the backport says if (readBytes() > 0)

Expected results:

The backported patch should match the code it is being backported from.

Additional info:

I built rpms from tomcat6-6.0.24-64.el6_5.src.rpm and verified that installing those rpms demonstrated the same crash in my application, which went away when downgrading to tomcat6-6.0.24-62. I then changed the bad zero comparison in the patch file and built rpms from that. Installing those rpms did fix the crash in my application.