Skip to content

Auto-port 4.2: MQTT: Make the decodeProperties early-REPLAY check actually fire#16919

Merged
chrisvest merged 1 commit into
4.2from
auto-port-pr-16813-to-4.2
Jun 5, 2026
Merged

Auto-port 4.2: MQTT: Make the decodeProperties early-REPLAY check actually fire#16919
chrisvest merged 1 commit into
4.2from
auto-port-pr-16813-to-4.2

Conversation

@netty-project-bot

Copy link
Copy Markdown
Contributor

Auto-port of #16813 to 4.2
Cherry-picked commit: 2d781e2


Motivation:

The CVE-2026-44248 fix in 82f47fa added a guard at the top of decodeProperties to trigger an early REPLAY when the cumulation buffer did not yet have the full properties block:

if (buffer.readableBytes() < totalPropertiesLength) {
    buffer.readSlice(totalPropertiesLength);
}

Because MqttDecoder extends ReplayingDecoder, the buffer passed to decodeProperties is a ReplayingDecoderByteBuf whose readableBytes() returns Integer.MAX_VALUE - readerIndex rather than the actual number of bytes available (see ReplayingDecoderByteBuf.readableBytes()). The if-condition is therefore false in practice and the readSlice() call never executes, so the early-REPLAY optimization is effectively dead code. When the properties block arrives in chunks the decoder still falls back to partial parsing followed by a mid-loop REPLAY, which defeats the optimization's intent on slow streams.

Modification:

Replace the broken readableBytes() check with a getByte() probe at buffer.readerIndex() + totalPropertiesLength - 1. ReplayingDecoderByteBuf.checkIndex throws Signal.REPLAY when index + 1 > writerIndex — i.e. exactly when the cumulation buffer does not yet hold the full properties block. The call has no side effect on readerIndex, so subsequent parsing is unchanged once the data arrives. A totalPropertiesLength > 0 guard skips the probe when there are no properties.

Result:

The early-REPLAY guard now actually fires when properties data is incomplete, restoring the CVE-2026-44248 fix's intent of avoiding repeated partial-properties parsing on slow streams. No semantic change on the happy path; all 107 existing codec-mqtt tests pass locally.

Note: this is complementary to #16787, which addresses the outer variable-header REPLAY handling broken by the same CVE patch (same buffer.readableBytes() antipattern in a ReplayingDecoder context). Both fixes are independent and can land in either order; together they restore the optimization the CVE-2026-44248 fix originally aimed for.

)

Motivation:

The CVE-2026-44248 fix in 82f47fa added
a guard at the top of `decodeProperties` to trigger an early REPLAY when
the cumulation buffer did not yet have the full properties block:

```java
if (buffer.readableBytes() < totalPropertiesLength) {
    buffer.readSlice(totalPropertiesLength);
}
```

Because `MqttDecoder` extends `ReplayingDecoder`, the buffer passed to
`decodeProperties` is a `ReplayingDecoderByteBuf` whose
`readableBytes()` returns `Integer.MAX_VALUE - readerIndex` rather than
the actual number of bytes available (see
`ReplayingDecoderByteBuf.readableBytes()`). The `if`-condition is
therefore false in practice and the `readSlice()` call never executes,
so the early-REPLAY optimization is effectively dead code. When the
properties block arrives in chunks the decoder still falls back to
partial parsing followed by a mid-loop REPLAY, which defeats the
optimization's intent on slow streams.

Modification:

Replace the broken `readableBytes()` check with a `getByte()` probe at
`buffer.readerIndex() + totalPropertiesLength - 1`.
`ReplayingDecoderByteBuf.checkIndex` throws `Signal.REPLAY` when `index
+ 1 > writerIndex` — i.e. exactly when the cumulation buffer does not
yet hold the full properties block. The call has no side effect on
`readerIndex`, so subsequent parsing is unchanged once the data arrives.
A `totalPropertiesLength > 0` guard skips the probe when there are no
properties.

Result:

The early-REPLAY guard now actually fires when properties data is
incomplete, restoring the CVE-2026-44248 fix's intent of avoiding
repeated partial-properties parsing on slow streams. No semantic change
on the happy path; all 107 existing `codec-mqtt` tests pass locally.

Note: this is complementary to #16787, which addresses the *outer*
variable-header REPLAY handling broken by the same CVE patch (same
`buffer.readableBytes()` antipattern in a ReplayingDecoder context).
Both fixes are independent and can land in either order; together they
restore the optimization the CVE-2026-44248 fix originally aimed for.

(cherry picked from commit 2d781e2)
@chrisvest chrisvest added this to the 4.2.16.Final milestone Jun 5, 2026
@chrisvest chrisvest enabled auto-merge (squash) June 5, 2026 17:39
@chrisvest chrisvest merged commit bfbdc4e into 4.2 Jun 5, 2026
19 checks passed
@chrisvest chrisvest deleted the auto-port-pr-16813-to-4.2 branch June 5, 2026 18:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants