Merge pull request #27 from cesarhernandezgt/tomcat-10.0.x-TT.x-patch…

…-http2-overhead

CVE-2023-44487

java/org/apache/coyote/http2/Http2AsyncParser.java

@@ -278,36 +278,39 @@ public void completed(Long result, Void attachment) {
                                 readUnknownFrame(streamId, frameTypeId, flags, payloadSize, payload);
                             }
                         }
-                        // See if there is a new 9 byte header and continue parsing if possible
-                        if (payload.remaining() >= 9) {
-                            int position = payload.position();
-                            payloadSize = ByteUtil.getThreeBytes(payload, position);
-                            frameTypeId = ByteUtil.getOneByte(payload, position + 3);
-                            frameType = FrameType.valueOf(frameTypeId);
-                            flags = ByteUtil.getOneByte(payload, position + 4);
-                            streamId = ByteUtil.get31Bits(payload, position + 5);
-                            streamException = false;
-                            if (payload.remaining() - 9 >= payloadSize) {
-                                continueParsing = true;
-                                // Now go over frame header
-                                payload.position(payload.position() + 9);
-                                try {
-                                    validateFrame(null, frameType, streamId, flags, payloadSize);
-                                } catch (StreamException e) {
-                                    error = e;
-                                    streamException = true;
-                                } catch (Http2Exception e) {
-                                    error = e;
-                                    continueParsing = false;
+                        if (!upgradeHandler.isOverheadLimitExceeded()) {
+                            // See if there is a new 9 byte header and continue parsing if possible
+                            if (payload.remaining() >= 9) {
+                                int position = payload.position();
+                                payloadSize = ByteUtil.getThreeBytes(payload, position);
+                                frameTypeId = ByteUtil.getOneByte(payload, position + 3);
+                                frameType = FrameType.valueOf(frameTypeId);
+                                flags = ByteUtil.getOneByte(payload, position + 4);
+                                streamId = ByteUtil.get31Bits(payload, position + 5);
+                                streamException = false;
+                                if (payload.remaining() - 9 >= payloadSize) {
+                                    continueParsing = true;
+                                    // Now go over frame header
+                                    payload.position(payload.position() + 9);
+                                    try {
+                                        validateFrame(null, frameType, streamId, flags, payloadSize);
+                                    } catch (StreamException e) {
+                                        error = e;
+                                        streamException = true;
+                                    } catch (Http2Exception e) {
+                                        error = e;
+                                        continueParsing = false;
+                                    }
                                 }
                             }
                         }
                     } while (continueParsing);
                 } catch (RuntimeException | IOException | Http2Exception e) {
                     error = e;
-                }
-                if (payload.hasRemaining()) {
-                    socketWrapper.unRead(payload);
+                } finally {
+                    if (payload.hasRemaining()) {
+                        socketWrapper.unRead(payload);
+                    }
                 }
             }
             if (state == CompletionState.DONE) {

java/org/apache/coyote/http2/Http2Protocol.java

@@ -54,8 +54,10 @@ public class Http2Protocol implements UpgradeProtocol {
     // Maximum amount of streams which can be concurrently executed over
     // a single connection
     static final int DEFAULT_MAX_CONCURRENT_STREAM_EXECUTION = 20;
-
+    // Default factor used when adjusting overhead count for overhead frames
     static final int DEFAULT_OVERHEAD_COUNT_FACTOR = 10;
+    // Default factor used when adjusting overhead count for reset frames
+    static final int DEFAULT_OVERHEAD_RESET_FACTOR = 50;
     // Not currently configurable. This makes the practical limit for
     // overheadCountFactor to be ~20. The exact limit will vary with traffic
     // patterns.
@@ -86,6 +88,7 @@ public class Http2Protocol implements UpgradeProtocol {
     private int maxHeaderCount = Constants.DEFAULT_MAX_HEADER_COUNT;
     private int maxTrailerCount = Constants.DEFAULT_MAX_TRAILER_COUNT;
     private int overheadCountFactor = DEFAULT_OVERHEAD_COUNT_FACTOR;
+    private int overheadResetFactor = DEFAULT_OVERHEAD_RESET_FACTOR;
     private int overheadContinuationThreshold = DEFAULT_OVERHEAD_CONTINUATION_THRESHOLD;
     private int overheadDataThreshold = DEFAULT_OVERHEAD_DATA_THRESHOLD;
     private int overheadWindowUpdateThreshold = DEFAULT_OVERHEAD_WINDOW_UPDATE_THRESHOLD;
@@ -292,6 +295,20 @@ public void setOverheadCountFactor(int overheadCountFactor) {
     }
 
 
+    public int getOverheadResetFactor() {
+        return overheadResetFactor;
+    }
+
+
+    public void setOverheadResetFactor(int overheadResetFactor) {
+        if (overheadResetFactor < 0) {
+            this.overheadResetFactor = 0;
+        } else {
+            this.overheadResetFactor = overheadResetFactor;
+        }
+    }
+
+
     public int getOverheadContinuationThreshold() {
         return overheadContinuationThreshold;
     }

java/org/apache/coyote/http2/Http2UpgradeHandler.java

@@ -380,7 +380,7 @@ public SocketState upgradeDispatch(SocketEvent status) {
                                 stream.close(se);
                             }
                         } finally {
-                            if (overheadCount.get() > 0) {
+                            if (isOverheadLimitExceeded()) {
                                 throw new ConnectionException(
                                         sm.getString("upgradeHandler.tooMuchOverhead", connectionId),
                                         Http2Error.ENHANCE_YOUR_CALM);
@@ -1494,6 +1494,11 @@ private void updateOverheadCount(FrameType frameType, int increment) {
     }
 
 
+    boolean isOverheadLimitExceeded() {
+        return overheadCount.get() > 0;
+    }
+
+
     // ----------------------------------------------- Http2Parser.Input methods
 
     @Override
@@ -1767,6 +1772,7 @@ public void reset(int streamId, long errorCode) throws Http2Exception  {
             log.debug(sm.getString("upgradeHandler.reset.receive", getConnectionId(), Integer.toString(streamId),
                     Long.toString(errorCode)));
         }
+        increaseOverheadCount(FrameType.RST, getProtocol().getOverheadResetFactor());
         AbstractNonZeroStream abstractNonZeroStream = getAbstractNonZeroStream(streamId, true);
         abstractNonZeroStream.checkState(FrameType.RST);
         if (abstractNonZeroStream instanceof Stream) {

modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/ConnectionPool.java

@@ -43,6 +43,8 @@
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.atomic.AtomicLong;
 
+import javax.sql.XAConnection;
+
 import org.apache.juli.logging.Log;
 import org.apache.juli.logging.LogFactory;
 
@@ -364,8 +366,10 @@ public Constructor<?> getProxyConstructor(boolean xa) throws NoSuchMethodExcepti
         //cache the constructor
         if (proxyClassConstructor == null ) {
             Class<?> proxyClass = xa ?
-                Proxy.getProxyClass(ConnectionPool.class.getClassLoader(), new Class[] {java.sql.Connection.class,javax.sql.PooledConnection.class, javax.sql.XAConnection.class}) :
-                Proxy.getProxyClass(ConnectionPool.class.getClassLoader(), new Class[] {java.sql.Connection.class,javax.sql.PooledConnection.class});
+                    Proxy.getProxyClass(ConnectionPool.class.getClassLoader(),
+                            new Class[] {Connection.class, javax.sql.PooledConnection.class, XAConnection.class}) :
+                    Proxy.getProxyClass(ConnectionPool.class.getClassLoader(),
+                            new Class[] {Connection.class, javax.sql.PooledConnection.class});
             proxyClassConstructor = proxyClass.getConstructor(new Class[] { InvocationHandler.class });
         }
         return proxyClassConstructor;

webapps/docs/changelog.xml

@@ -105,6 +105,15 @@
   issues do not "pop up" wrt. others).
 -->
 <section name="Tomcat 10.0.28 (markt)" rtext="in development">
+  <subsection name="jdbc-pool">
+    <changelog>
+      <fix>
+        <bug>67664</bug>: Correct a regression in the clean-up of unencessary
+        use of fully qualified class names in 10.1.14 that broke the jdbc-pool.
+        (markt)
+      </fix>
+    </changelog>
+  </subsection>
   <subsection name="Other">
     <changelog>
       <update>
@@ -157,6 +166,9 @@
       <fix>
         Align validation of HTTP trailer fields with standard fields. (markt)
       </fix>
+      <fix>
+        Improvements to HTTP/2 overhead protection. (markt)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Jasper">

webapps/docs/config/http2.xml

@@ -139,14 +139,21 @@
       count starts at <code>-10 * overheadCountFactor</code>. The count is
       decreased by 20 for each data frame sent or received and each headers frame
       received. The count is increased by the <code>overheadCountFactor</code>
-      for each setting received, priority frame received and ping received. If
+      for each setting, priority, priority update and ping frame received. If
       the overhead count exceeds zero, the connection is closed. A value of less
       than <code>1</code> disables this protection. In normal usage a value of
       approximately <code>20</code> or higher will close the connection before
       any streams can complete. If not specified, a default value of
       <code>10</code> will be used.</p>
     </attribute>
 
+    <attribute name="overheadResetFactor" required="false">
+      <p>The amount by which the overhead count (see
+      <strong>overheadCountFactor</strong>) will be increased for each reset
+      frame received. If not specified, a default value of <code>50</code> will
+      be used. A value of less than zero will be treated as zero.</p>
+    </attribute>
+
     <attribute name="overheadDataThreshold" required="false">
       <p>The threshold below which the average payload size of the current and
       previous non-final <code>DATA</code> frames will trigger an increase in