CVE-2017-5651

@@ -58,6 +58,7 @@
 import org.apache.tomcat.util.net.AbstractEndpoint.Handler.SocketState;
 import org.apache.tomcat.util.net.SSLSupport;
 import org.apache.tomcat.util.net.SendfileDataBase;
+import org.apache.tomcat.util.net.SendfileState;
 import org.apache.tomcat.util.net.SocketWrapperBase;
 import org.apache.tomcat.util.res.StringManager;
 
@@ -671,9 +672,10 @@ public SocketState service(SocketWrapperBase<?> socketWrapper)
         openSocket = false;
         readComplete = true;
         boolean keptAlive = false;
+        SendfileState sendfileState = SendfileState.DONE;
 
-        while (!getErrorState().isError() && keepAlive && !isAsync() &&
-                upgradeToken == null && !endpoint.isPaused()) {
+        while (!getErrorState().isError() && keepAlive && !isAsync() && upgradeToken == null &&
+                sendfileState == SendfileState.DONE && !endpoint.isPaused()) {
 
             // Parsing the request header
             try {
@@ -862,9 +864,7 @@ public SocketState service(SocketWrapperBase<?> socketWrapper)
 
             rp.setStage(org.apache.coyote.Constants.STAGE_KEEPALIVE);
 
-            if (breakKeepAliveLoop(socketWrapper)) {
-                break;
-            }
+            sendfileState = processSendfile(socketWrapper);
         }
 
         rp.setStage(org.apache.coyote.Constants.STAGE_ENDED);
@@ -876,7 +876,7 @@ public SocketState service(SocketWrapperBase<?> socketWrapper)
         } else if (isUpgrade()) {
             return SocketState.UPGRADING;
         } else {
-            if (sendfileData != null) {
+            if (sendfileState == SendfileState.PENDING) {
                 return SocketState.SENDFILE;
             } else {
                 if (openSocket) {
@@ -952,7 +952,6 @@ private void prepareRequest() {
         http11 = true;
         http09 = false;
         contentDelimitation = false;
-        sendfileData = null;
 
         if (endpoint.isSSLEnabled()) {
             request.scheme().setString("https");
@@ -1159,15 +1158,14 @@ protected final void prepareResponse() throws IOException {
         }
 
         // Sendfile support
-        boolean sendingWithSendfile = false;
         if (endpoint.getUseSendfile()) {
-            sendingWithSendfile = prepareSendfile(outputFilters);
+            prepareSendfile(outputFilters);
         }
 
         // Check for compression
         boolean isCompressible = false;
         boolean useCompression = false;
-        if (entityBody && (compressionLevel > 0) && !sendingWithSendfile) {
+        if (entityBody && (compressionLevel > 0) && sendfileData == null) {
             isCompressible = isCompressible();
             if (isCompressible) {
                 useCompression = useCompression();
@@ -1309,10 +1307,12 @@ private static boolean isConnectionClose(MimeHeaders headers) {
         return connection.equals(Constants.CLOSE);
     }
 
-    private boolean prepareSendfile(OutputFilter[] outputFilters) {
+    private void prepareSendfile(OutputFilter[] outputFilters) {
         String fileName = (String) request.getAttribute(
                 org.apache.coyote.Constants.SENDFILE_FILENAME_ATTR);
-        if (fileName != null) {
+        if (fileName == null) {
+            sendfileData = null;
+        } else {
             // No entity body sent here
             outputBuffer.addActiveFilter(outputFilters[Constants.VOID_FILTER]);
             contentDelimitation = true;
@@ -1321,9 +1321,7 @@ private boolean prepareSendfile(OutputFilter[] outputFilters) {
             long end = ((Long) request.getAttribute(
                     org.apache.coyote.Constants.SENDFILE_FILE_END_ATTR)).longValue();
             sendfileData = socketWrapper.createSendfileData(fileName, pos, end - pos);
-            return true;
         }
-        return false;
     }
 
     /**
@@ -1604,34 +1602,31 @@ public boolean isUpgrade() {
 
 
     /**
-     * Checks to see if the keep-alive loop should be broken, performing any
-     * processing (e.g. sendfile handling) that may have an impact on whether
-     * or not the keep-alive loop should be broken.
+     * Trigger sendfile processing if required.
      *
-     * @return true if the keep-alive loop should be broken
+     * @return The state of send file processing
      */
-    private boolean breakKeepAliveLoop(SocketWrapperBase<?> socketWrapper) {
+    private SendfileState processSendfile(SocketWrapperBase<?> socketWrapper) {
         openSocket = keepAlive;
+        // Done is equivalent to sendfile not being used
+        SendfileState result = SendfileState.DONE;
         // Do sendfile as needed: add socket to sendfile and end
         if (sendfileData != null && !getErrorState().isError()) {
             sendfileData.keepAlive = keepAlive;
-            switch (socketWrapper.processSendfile(sendfileData)) {
-            case DONE:
-                // If sendfile is complete, no need to break keep-alive loop
-                sendfileData = null;
-                return false;
-            case PENDING:
-                return true;
+            result = socketWrapper.processSendfile(sendfileData);
+            switch (result) {
             case ERROR:
                 // Write failed
                 if (log.isDebugEnabled()) {
                     log.debug(sm.getString("http11processor.sendfile.error"));
                 }
                 setErrorState(ErrorState.CLOSE_CONNECTION_NOW, null);
-                return true;
+                //$FALL-THROUGH$
+            default:
+                sendfileData = null;
             }
         }
-        return false;
+        return result;
     }
 
 

@@ -107,6 +107,11 @@
         Improve HPACK specification compliance by fixing some test failures
         reported by the h2spec tool written by Moto Ishizawa. (markt)
       </fix>
+      <fix>
+        <bug>60918</bug>: Fix sendfile processing error that could lead to
+        subsequent requests experiencing and <code>IllegalStateException</code>.
+        (markt)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Jasper">

@@ -55,6 +55,7 @@
 import org.apache.tomcat.util.net.AbstractEndpoint.Handler.SocketState;
 import org.apache.tomcat.util.net.SSLSupport;
 import org.apache.tomcat.util.net.SendfileDataBase;
+import org.apache.tomcat.util.net.SendfileState;
 import org.apache.tomcat.util.net.SocketWrapperBase;
 import org.apache.tomcat.util.res.StringManager;
 
@@ -368,9 +369,10 @@ public SocketState service(SocketWrapperBase<?> socketWrapper)
         openSocket = false;
         readComplete = true;
         boolean keptAlive = false;
+        SendfileState sendfileState = SendfileState.DONE;
 
-        while (!getErrorState().isError() && keepAlive && !isAsync() &&
-                upgradeToken == null && !protocol.isPaused()) {
+        while (!getErrorState().isError() && keepAlive && !isAsync() && upgradeToken == null &&
+                sendfileState == SendfileState.DONE && !protocol.isPaused()) {
 
             // Parsing the request header
             try {
@@ -561,9 +563,7 @@ public SocketState service(SocketWrapperBase<?> socketWrapper)
 
             rp.setStage(org.apache.coyote.Constants.STAGE_KEEPALIVE);
 
-            if (breakKeepAliveLoop(socketWrapper)) {
-                break;
-            }
+            sendfileState = processSendfile(socketWrapper);
         }
 
         rp.setStage(org.apache.coyote.Constants.STAGE_ENDED);
@@ -575,7 +575,7 @@ public SocketState service(SocketWrapperBase<?> socketWrapper)
         } else if (isUpgrade()) {
             return SocketState.UPGRADING;
         } else {
-            if (sendfileData != null) {
+            if (sendfileState == SendfileState.PENDING) {
                 return SocketState.SENDFILE;
             } else {
                 if (openSocket) {
@@ -651,7 +651,6 @@ private void prepareRequest() {
         http11 = true;
         http09 = false;
         contentDelimitation = false;
-        sendfileData = null;
 
         if (protocol.isSSLEnabled()) {
             request.scheme().setString("https");
@@ -858,15 +857,14 @@ protected final void prepareResponse() throws IOException {
         }
 
         // Sendfile support
-        boolean sendingWithSendfile = false;
         if (protocol.getUseSendfile()) {
-            sendingWithSendfile = prepareSendfile(outputFilters);
+            prepareSendfile(outputFilters);
         }
 
         // Check for compression
         boolean isCompressible = false;
         boolean useCompression = false;
-        if (entityBody && (protocol.getCompressionLevel() > 0) && !sendingWithSendfile) {
+        if (entityBody && (protocol.getCompressionLevel() > 0) && sendfileData == null) {
             isCompressible = isCompressible();
             if (isCompressible) {
                 useCompression = useCompression();
@@ -1009,10 +1007,12 @@ private static boolean isConnectionClose(MimeHeaders headers) {
         return connection.equals(Constants.CLOSE);
     }
 
-    private boolean prepareSendfile(OutputFilter[] outputFilters) {
+    private void prepareSendfile(OutputFilter[] outputFilters) {
         String fileName = (String) request.getAttribute(
                 org.apache.coyote.Constants.SENDFILE_FILENAME_ATTR);
-        if (fileName != null) {
+        if (fileName == null) {
+            sendfileData = null;
+        } else {
             // No entity body sent here
             outputBuffer.addActiveFilter(outputFilters[Constants.VOID_FILTER]);
             contentDelimitation = true;
@@ -1021,9 +1021,7 @@ private boolean prepareSendfile(OutputFilter[] outputFilters) {
             long end = ((Long) request.getAttribute(
                     org.apache.coyote.Constants.SENDFILE_FILE_END_ATTR)).longValue();
             sendfileData = socketWrapper.createSendfileData(fileName, pos, end - pos);
-            return true;
         }
-        return false;
     }
 
     /**
@@ -1304,34 +1302,31 @@ public boolean isUpgrade() {
 
 
     /**
-     * Checks to see if the keep-alive loop should be broken, performing any
-     * processing (e.g. sendfile handling) that may have an impact on whether
-     * or not the keep-alive loop should be broken.
+     * Trigger sendfile processing if required.
      *
-     * @return true if the keep-alive loop should be broken
+     * @return The state of send file processing
      */
-    private boolean breakKeepAliveLoop(SocketWrapperBase<?> socketWrapper) {
+    private SendfileState processSendfile(SocketWrapperBase<?> socketWrapper) {
         openSocket = keepAlive;
+        // Done is equivalent to sendfile not being used
+        SendfileState result = SendfileState.DONE;
         // Do sendfile as needed: add socket to sendfile and end
         if (sendfileData != null && !getErrorState().isError()) {
             sendfileData.keepAlive = keepAlive;
-            switch (socketWrapper.processSendfile(sendfileData)) {
-            case DONE:
-                // If sendfile is complete, no need to break keep-alive loop
-                sendfileData = null;
-                return false;
-            case PENDING:
-                return true;
+            result = socketWrapper.processSendfile(sendfileData);
+            switch (result) {
             case ERROR:
                 // Write failed
                 if (log.isDebugEnabled()) {
                     log.debug(sm.getString("http11processor.sendfile.error"));
                 }
                 setErrorState(ErrorState.CLOSE_CONNECTION_NOW, null);
-                return true;
+                //$FALL-THROUGH$
+            default:
+                sendfileData = null;
             }
         }
-        return false;
+        return result;
     }
 
 

@@ -132,6 +132,11 @@
         Improve HPACK specification compliance by fixing some test failures
         reported by the h2spec tool written by Moto Ishizawa. (markt)
       </fix>
+      <fix>
+        <bug>60918</bug>: Fix sendfile processing error that could lead to
+        subsequent requests experiencing and <code>IllegalStateException</code>.
+        (markt)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Jasper">