Author: markt
Date: Tue Aug 16 12:50:33 2011
New Revision: 1158244

URL: http://svn.apache.org/viewvc?rev=1158244&view=rev
Log:
Fix various sendfile issues
CVE-2011-2526

Modified:
    tomcat/tc5.5.x/trunk/STATUS.txt
    
tomcat/tc5.5.x/trunk/connectors/http11/src/java/org/apache/coyote/http11/Http11AprProcessor.java
    
tomcat/tc5.5.x/trunk/connectors/http11/src/java/org/apache/coyote/http11/LocalStrings.properties
    
tomcat/tc5.5.x/trunk/connectors/util/java/org/apache/tomcat/util/net/AprEndpoint.java
    
tomcat/tc5.5.x/trunk/container/catalina/src/share/org/apache/catalina/connector/LocalStrings.properties
    
tomcat/tc5.5.x/trunk/container/catalina/src/share/org/apache/catalina/connector/Request.java
    
tomcat/tc5.5.x/trunk/container/catalina/src/share/org/apache/catalina/servlets/DefaultServlet.java
    tomcat/tc5.5.x/trunk/container/webapps/docs/changelog.xml

Modified: tomcat/tc5.5.x/trunk/STATUS.txt
URL: 
http://svn.apache.org/viewvc/tomcat/tc5.5.x/trunk/STATUS.txt?rev=1158244&r1=1158243&r2=1158244&view=diff
==============================================================================
--- tomcat/tc5.5.x/trunk/STATUS.txt (original)
+++ tomcat/tc5.5.x/trunk/STATUS.txt Tue Aug 16 12:50:33 2011
@@ -94,12 +94,6 @@ PATCHES PROPOSED TO BACKPORT:
        I think there is not much demand for this feature in 5.5 to justify 
this.
        The rest of changes and fixes are OK to backport (e.g. allow to specify 
port numbers).
 
-* Fix various sendfile issues. CVE-2011-2526
-  This is a port of r1145380, r1145694 and r1146005
-  http://people.apache.org/~markt/patches/2011-07-13-cve-2011-2526-tc5.patch
-  +1: markt, kfujino, kkolinko
-  -1:
-
 * Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=41179
   Return 404 rather than 400 if no ROOT context is deployed
   http://people.apache.org/~markt/patches/2011-07-22-bug41179-tc5.patch

Modified: 
tomcat/tc5.5.x/trunk/connectors/http11/src/java/org/apache/coyote/http11/Http11AprProcessor.java
URL: 
http://svn.apache.org/viewvc/tomcat/tc5.5.x/trunk/connectors/http11/src/java/org/apache/coyote/http11/Http11AprProcessor.java?rev=1158244&r1=1158243&r2=1158244&view=diff
==============================================================================
--- 
tomcat/tc5.5.x/trunk/connectors/http11/src/java/org/apache/coyote/http11/Http11AprProcessor.java
 (original)
+++ 
tomcat/tc5.5.x/trunk/connectors/http11/src/java/org/apache/coyote/http11/Http11AprProcessor.java
 Tue Aug 16 12:50:33 2011
@@ -907,7 +907,18 @@ public class Http11AprProcessor implemen
                 sendfileData.socket = socket;
                 sendfileData.keepAlive = keepAlive;
                 if (!endpoint.getSendfile().add(sendfileData)) {
-                    openSocket = true;
+                    if (sendfileData.socket == 0) {
+                        // Didn't send all the data but the socket is no longer
+                        // set. Something went wrong. Close the connection.
+                        // Too late to set status code.
+                        if (log.isDebugEnabled()) {
+                            log.debug(sm.getString(
+                                    "http11processor.sendfile.error"));
+                        }
+                        openSocket = false;
+                    } else {
+                        openSocket = true;
+                    }
                     break;
                 }
             }

Modified: 
tomcat/tc5.5.x/trunk/connectors/http11/src/java/org/apache/coyote/http11/LocalStrings.properties
URL: 
http://svn.apache.org/viewvc/tomcat/tc5.5.x/trunk/connectors/http11/src/java/org/apache/coyote/http11/LocalStrings.properties?rev=1158244&r1=1158243&r2=1158244&view=diff
==============================================================================
--- 
tomcat/tc5.5.x/trunk/connectors/http11/src/java/org/apache/coyote/http11/LocalStrings.properties
 (original)
+++ 
tomcat/tc5.5.x/trunk/connectors/http11/src/java/org/apache/coyote/http11/LocalStrings.properties
 Tue Aug 16 12:50:33 2011
@@ -56,6 +56,7 @@ http11processor.response.finish=Error fi
 http11processor.socket.info=Exception getting socket information
 http11processor.socket.ssl=Exception getting SSL attributes
 http11processor.socket.timeout=Error setting socket timeout
+http11processor.sendfile.error=Error sending data using sendfile. May be 
caused by invalid request attributes for start/end points
 
 #
 # InternalInputBuffer

Modified: 
tomcat/tc5.5.x/trunk/connectors/util/java/org/apache/tomcat/util/net/AprEndpoint.java
URL: 
http://svn.apache.org/viewvc/tomcat/tc5.5.x/trunk/connectors/util/java/org/apache/tomcat/util/net/AprEndpoint.java?rev=1158244&r1=1158243&r2=1158244&view=diff
==============================================================================
--- 
tomcat/tc5.5.x/trunk/connectors/util/java/org/apache/tomcat/util/net/AprEndpoint.java
 (original)
+++ 
tomcat/tc5.5.x/trunk/connectors/util/java/org/apache/tomcat/util/net/AprEndpoint.java
 Tue Aug 16 12:50:33 2011
@@ -1429,7 +1429,9 @@ public class AprEndpoint {
                                                data.pos, data.end - data.pos, 
0);
                     if (nw < 0) {
                         if (!(-nw == Status.EAGAIN)) {
-                            Socket.destroy(data.socket);
+                            Pool.destroy(data.fdpool);
+                            // No need to close socket, this will be done by
+                            // calling code since data.socket == 0
                             data.socket = 0;
                             return false;
                         } else {

Modified: 
tomcat/tc5.5.x/trunk/container/catalina/src/share/org/apache/catalina/connector/LocalStrings.properties
URL: 
http://svn.apache.org/viewvc/tomcat/tc5.5.x/trunk/container/catalina/src/share/org/apache/catalina/connector/LocalStrings.properties?rev=1158244&r1=1158243&r2=1158244&view=diff
==============================================================================
--- 
tomcat/tc5.5.x/trunk/container/catalina/src/share/org/apache/catalina/connector/LocalStrings.properties
 (original)
+++ 
tomcat/tc5.5.x/trunk/container/catalina/src/share/org/apache/catalina/connector/LocalStrings.properties
 Tue Aug 16 12:50:33 2011
@@ -60,6 +60,7 @@ coyoteRequest.parseParameters=Exception 
 coyoteRequest.postTooLarge=Parameters were not parsed because the size of the 
posted data was too big. Use the maxPostSize attribute of the connector to 
resolve this if the application should accept large POSTs.
 coyoteRequest.chunkedPostTooLarge=Parameters were not parsed because the size 
of the posted data was too big. Because this request was a chunked request, it 
could not be processed further. Use the maxPostSize attribute of the connector 
to resolve this if the application should accept large POSTs.
 coyoteRequest.sessionEndAccessFail=Exception triggered ending access to 
session while recycling request
+coyoteRequest.sendfileNotCanonical=Unable to determine canonical name of file 
[{0}] specified for use with sendfile
 
 requestFacade.nullRequest=Null request object
 responseFacade.nullResponse=Null response object

Modified: 
tomcat/tc5.5.x/trunk/container/catalina/src/share/org/apache/catalina/connector/Request.java
URL: 
http://svn.apache.org/viewvc/tomcat/tc5.5.x/trunk/container/catalina/src/share/org/apache/catalina/connector/Request.java?rev=1158244&r1=1158243&r2=1158244&view=diff
==============================================================================
--- 
tomcat/tc5.5.x/trunk/container/catalina/src/share/org/apache/catalina/connector/Request.java
 (original)
+++ 
tomcat/tc5.5.x/trunk/container/catalina/src/share/org/apache/catalina/connector/Request.java
 Tue Aug 16 12:50:33 2011
@@ -19,6 +19,7 @@
 package org.apache.catalina.connector;
 
 
+import java.io.File;
 import java.io.InputStream;
 import java.io.IOException;
 import java.io.BufferedReader;
@@ -1374,6 +1375,27 @@ public class Request
             return;
         }
 
+        if (System.getSecurityManager() != null &&
+                name.equals("org.apache.tomcat.sendfile.filename")) {
+            // Use the canonical file name to avoid any possible symlink and
+            // relative path issues
+            String canonicalPath;
+            try {
+                canonicalPath = new File(value.toString()).getCanonicalPath();
+            } catch (IOException e) {
+                SecurityException se = new SecurityException(sm.getString(
+                        "coyoteRequest.sendfileNotCanonical", value));
+                se.initCause(e);
+                throw se;
+            }
+            // Sendfile is performed in Tomcat's security context so need to
+            // check if the web app is permitted to access the file while still
+            // in the web app's security context
+            System.getSecurityManager().checkRead(canonicalPath);
+            // Update the value so the canonical path is used
+            value = canonicalPath;
+        }
+
         Object oldValue = null;
         boolean replaced = false;
 

Modified: 
tomcat/tc5.5.x/trunk/container/catalina/src/share/org/apache/catalina/servlets/DefaultServlet.java
URL: 
http://svn.apache.org/viewvc/tomcat/tc5.5.x/trunk/container/catalina/src/share/org/apache/catalina/servlets/DefaultServlet.java?rev=1158244&r1=1158243&r2=1158244&view=diff
==============================================================================
--- 
tomcat/tc5.5.x/trunk/container/catalina/src/share/org/apache/catalina/servlets/DefaultServlet.java
 (original)
+++ 
tomcat/tc5.5.x/trunk/container/catalina/src/share/org/apache/catalina/servlets/DefaultServlet.java
 Tue Aug 16 12:50:33 2011
@@ -1639,7 +1639,6 @@ public class DefaultServlet
                 request.setAttribute("org.apache.tomcat.sendfile.start", new 
Long(range.start));
                 request.setAttribute("org.apache.tomcat.sendfile.end", new 
Long(range.end + 1));
             }
-            request.setAttribute("org.apache.tomcat.sendfile.token", this);
             return true;
         } else {
             return false;

Modified: tomcat/tc5.5.x/trunk/container/webapps/docs/changelog.xml
URL: 
http://svn.apache.org/viewvc/tomcat/tc5.5.x/trunk/container/webapps/docs/changelog.xml?rev=1158244&r1=1158243&r2=1158244&view=diff
==============================================================================
--- tomcat/tc5.5.x/trunk/container/webapps/docs/changelog.xml (original)
+++ tomcat/tc5.5.x/trunk/container/webapps/docs/changelog.xml Tue Aug 16 
12:50:33 2011
@@ -76,6 +76,14 @@
       </fix>
     </changelog>
   </subsection>
+  <subsection name="Coyote">
+    <changelog>
+      <fix>
+        Fix CVE-2011-2526. Protect against crashes (HTTP APR) if sendfile is
+        configured to send more data than is available in the file. (markt) 
+      </fix>
+    </changelog>
+  </subsection>
   <subsection name="Jasper">
     <changelog>
       <fix>



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Reply via email to