Skip to content

[UNDERTOW-2598] CVE-2025-9784 Prevent a MadeYouReset HTTP2 attack by … #1778

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+98 −19
Open

[UNDERTOW-2598] CVE-2025-9784 Prevent a MadeYouReset HTTP2 attack by … #1778

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 41 additions & 11 deletions core/src/main/java/io/undertow/protocols/http2/Http2Channel.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,9 +180,13 @@ public class Http2Channel extends AbstractFramedChannel<Http2Channel, AbstractHt
// the time window for counting rst frames received
private final long rstFramesTimeWindow;
// the time in milliseconds the last rst frame was received
private long lastRstFrameMillis = System.currentTimeMillis();
// the total number of received rst frames during current time windows
private long lastReceivedRstFrameMillis = System.currentTimeMillis();
// the time in milliseconds the last rst frame was sent
private long lastSentRstFrameMillis = System.currentTimeMillis();
// the total number of received rst frames during current time windows
private int receivedRstFramesPerWindow;
// the total number of sent rst frames during current time windows
private int sentRstFramesPerWindow;


private static final AtomicIntegerFieldUpdater<Http2Channel> sendConcurrentStreamsAtomicUpdater = AtomicIntegerFieldUpdater.newUpdater(
Expand Down Expand Up @@ -1210,11 +1214,14 @@ public void sendRstStream(int streamId, int statusCode) {
return;
}
handleRstStream(streamId, false);
if(UndertowLogger.REQUEST_IO_LOGGER.isDebugEnabled()) {
UndertowLogger.REQUEST_IO_LOGGER.debugf(new ClosedChannelException(), "Sending rststream on channel %s stream %s", this, streamId);
if (!this.isThisGoneAway()) {
if (UndertowLogger.REQUEST_IO_LOGGER.isDebugEnabled()) {
UndertowLogger.REQUEST_IO_LOGGER.debugf(new ClosedChannelException(),
"Sending rststream on channel %s stream %s", this, streamId);
}
Http2RstStreamSinkChannel channel = new Http2RstStreamSinkChannel(this, streamId, statusCode);
flushChannelIgnoreFailure(channel);
}
Http2RstStreamSinkChannel channel = new Http2RstStreamSinkChannel(this, streamId, statusCode);
flushChannelIgnoreFailure(channel);
}

private StreamHolder handleRstStream(int streamId, boolean receivedRst) {
Expand All @@ -1241,9 +1248,12 @@ private StreamHolder handleRstStream(int streamId, boolean receivedRst) {
//NOTE: this is specific case when its set.
holder.resetByPeer = receivedRst;
} else {
handleRstWindow();
trackReceivedRstWindow();
}
}
else {
trackSentRstWindow();
}
} else if(receivedRst){
final StreamHolder resetStream = resetStreamTracker.find(streamId, true);
if(resetStream != null && resetStream.resetByPeer) {
Expand All @@ -1252,17 +1262,19 @@ private StreamHolder handleRstStream(int streamId, boolean receivedRst) {
//cause other end to flare up with RST, this RST can be safely ignored.
//TODO: do we need to check error code?
} else {
handleRstWindow();
trackReceivedRstWindow();
}
} else {
trackSentRstWindow();
}
return holder;
}

private void handleRstWindow() {
private void trackReceivedRstWindow() {
long currentTimeMillis = System.currentTimeMillis();
// reset the window tracking
if (currentTimeMillis - lastRstFrameMillis >= rstFramesTimeWindow) {
lastRstFrameMillis = currentTimeMillis;
if (currentTimeMillis - lastReceivedRstFrameMillis >= rstFramesTimeWindow) {
lastReceivedRstFrameMillis = currentTimeMillis;
receivedRstFramesPerWindow = 1;
} else {
receivedRstFramesPerWindow++;
Expand All @@ -1275,6 +1287,24 @@ private void handleRstWindow() {
}
}

private void trackSentRstWindow() {
long currentTimeMillis = System.currentTimeMillis();
// reset the window tracking
if (currentTimeMillis - lastSentRstFrameMillis >= rstFramesTimeWindow) {
lastSentRstFrameMillis = currentTimeMillis;
sentRstFramesPerWindow = 1;
} else {
sentRstFramesPerWindow++;
if (sentRstFramesPerWindow > maxRstFramesPerWindow) {
sendGoAway(Http2Channel.ERROR_ENHANCE_YOUR_CALM);
UndertowLogger.REQUEST_IO_LOGGER.debugf(
"Reached maximum number of sent rst frames %s during %s ms, sending GO_AWAY 11",
maxRstFramesPerWindow, rstFramesTimeWindow);
IoUtils.safeClose(this);
}
}
}

/**
* Creates a response stream to respond to the initial HTTP upgrade
*
Expand Down
13 changes: 11 additions & 2 deletions core/src/main/java/io/undertow/protocols/http2/Http2FramePriority.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,17 @@ public void frameAdded(AbstractHttp2StreamSinkChannel addedFrame, List<AbstractH
}
}

if (pending instanceof Http2StreamSinkChannel) {
SendFrameHeader header = ((Http2StreamSinkChannel) pending).generateSendFrameHeader();
if (pending.isOpen() && pending instanceof Http2StreamSinkChannel) {
final SendFrameHeader header;
try {
header = ((Http2StreamSinkChannel) pending).generateSendFrameHeader();
} catch (IllegalStateException e) {
if (pending.isOpen()) {
throw e;
} else {
continue;
}
}
if (header.getByteBuffer() != null) {
pendingFrames.add(pending);
it.remove();
Expand Down
34 changes: 29 additions & 5 deletions core/src/main/java/io/undertow/server/protocol/framed/AbstractFramedChannel.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -497,6 +497,12 @@ public synchronized R receive() throws IOException {
partialRead = true;
}
return null;
} catch (IllegalStateException e) {
if (!pooled.isFreed()) {
throw e;
}
channel.close();
return null;
} catch (IOException|RuntimeException|Error e) {
//something has code wrong with parsing, close the read side
//we don't close the write side, as the underlying implementation will most likely want to send an error
Expand Down Expand Up @@ -524,13 +530,21 @@ public synchronized R receive() throws IOException {
}
}
if(requiresReinvoke) {
ByteBuffer buffer = null;
if(pooled != null && !pooled.isFreed()) {
if(pooled.getBuffer().remaining() == reinvokeDataRemaining) {
pooled.close();
readData = null;
UndertowLogger.REQUEST_IO_LOGGER.debugf("Partial message read before connection close %s", this);
try {
buffer = pooled.getBuffer();
} catch (IllegalStateException e) {
if (!pooled.isFreed()) {
throw e;
}
}
}
if (buffer != null && buffer.remaining() == reinvokeDataRemaining) {
pooled.close();
readData = null;
UndertowLogger.REQUEST_IO_LOGGER.debugf("Partial message read before connection close %s", this);
}
channel.getSourceChannel().wakeupReads();
}
// race condition, asynchronous close while reading
Expand Down Expand Up @@ -1040,7 +1054,17 @@ public void handleEvent(final CloseableChannel c) {
@Override
public void run() {
while (localReadData != null && !localReadData.isFreed()) {
int rem = localReadData.getBuffer().remaining();
final ByteBuffer buffer;
try {
buffer = localReadData.getBuffer();
} catch (IllegalStateException e) {
if (localReadData.isFreed()) {
return;
} else {
throw e;
}
}
int rem = buffer.remaining();
ChannelListener listener = receiveSetter.get();
if(listener == null) {
listener = DRAIN_LISTENER;
Expand Down
11 changes: 10 additions & 1 deletion core/src/main/java/io/undertow/server/protocol/framed/AbstractFramedStreamSinkChannel.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -429,7 +429,16 @@ public int write(ByteBuffer src) throws IOException {
if(writeBuffer == null) {
writeBuffer = getChannel().getBufferPool().allocate();
}
ByteBuffer buffer = writeBuffer.getBuffer();
final ByteBuffer buffer;
try {
buffer = writeBuffer.getBuffer();
} catch (IllegalStateException | NullPointerException e) {
if (!safeToSend()) {
return 0;
}
throw e;
}

int copied = Buffers.copy(buffer, src);
if(!buffer.hasRemaining()) {
handleBufferFull();
Expand Down
7 changes: 7 additions & 0 deletions spotbugs-exclude.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,13 @@
<Method name="isClient"/>
</Match>

<!--NPE can occur because an asynchronous close could happen concurrently with write on exceptional cases -->
<Match>
<Bug pattern="DCN_NULLPOINTER_EXCEPTION"/>
<Class name="io.undertow.server.protocol.framed.AbstractFramedStreamSinkChannel"/>
<Method name="write"/>
</Match>

<!-- Even per javadoc of Object.wait(), this should be always used in loop. It is on purpose -->
<Match>
<Bug pattern="WA_NOT_IN_LOOP"/>
Expand Down
Loading