fix: enhance token server request handling and add max frame length validation to prevent memory issues

[yungyu16]

中文版本

请描述这个PR的作用以及为什么需要它

本PR增强了token服务器的请求处理能力，并增加了最大帧长度验证，以防止潜在的安全风险和性能问题。

当公司安全团队使用nmap进行端口扫描时，可能会向token服务器发送畸形数据包。

这些数据包可能包含异常大的长度字段，导致服务器创建极大的字节数组，从而引发过多的内存消耗和Full GC问题。

现象

在maxFrameLength最大为1024时，解码ping报文时，会创建16M的临时数组，带来内存压力。

复现

1. 本地启动 com.alibaba.csp.sentinel.demo.cluster.ClusterServerDemo
2. 本地安装nmap命令行工具 brew install nmap
3. 本地执行napp扫描脚本 nmap -oX - 127.0.0.1 -p 11111 -T4 -sT -sV -Pn -n --host-timeout 300000ms --max-retries 1 --min-parallelism 16 --max-scan-delay 5s

4. 在 com.alibaba.csp.sentinel.cluster.server.codec.data.PingRequestDataDecoder.decode 断点，可以看到解码出超大的length

原理

根据namp端口特征 规则库可以看到，DNSVersionBindReqTCP 类型的探测报文会被token server误解码为ping包。
https://raw.githubusercontent.com/nmap/nmap/refs/heads/master/nmap-service-probes

这个PR是否修复了某个问题？

Fixes #3636

修复畸形报文中的长度或数量字段导致过度内存分配、越界读取，以及残留字节影响后续解码的问题。

请描述您是如何解决的

1. 为 Netty 外层帧、ParamFlow 参数数量和字符串长度设置独立上限。
2. Ping 解码在分配内存前校验长度非负且与当前 frame 的剩余字节数完全一致。
3. ParamFlow 解码在每次读取前校验数量、类型、字段长度和剩余字节，并拒绝尾随字节。
4. 协议解码异常统一收口到 exceptionCaught 并关闭连接。

请描述如何验证这个PR

新增并执行 43 个相关测试，覆盖：

1. 正常及空 Ping/ParamFlow 请求。
2. 负数、超限、与剩余字节不匹配及截断的长度/数量字段。
3. 未知参数类型、尾随字节、空 payload 和超长外层 frame。
4. 协议异常和超长 frame 均关闭连接。

特别说明（给评审人员）

此修复解决了畸形数据包可能导致过度内存分配的潜在安全和性能问题。该解决方案引入了适当的数据包大小验证和限制，以防止拒绝服务场景的发生。

English Version

Describe what this PR does / why we need it

This PR enhances the token server's request handling and adds max frame length validation to prevent potential security risks and performance issues. When the company's security team performs port scanning using nmap, malformed packets may be sent to the token server. These packets may contain abnormally large length fields, which could cause the server to create extremely large byte arrays, leading to excessive memory consumption and Full GC issues.

Phenomenon

When maxFrameLength is set to a maximum of 1024, decoding ping packets creates a 16M temporary array, causing memory pressure.

Reproduction Steps

1. Start locally: com.alibaba.csp.sentinel.demo.cluster.ClusterServerDemo
2. Install nmap command line tool locally: brew install nmap
3. Execute nmap scanning script locally: nmap -oX - 127.0.0.1 -p 11111 -T4 -sT -sV -Pn -n --host-timeout 300000ms --max-retries 1 --min-parallelism 16 --max-scan-delay 5s

4. Set a breakpoint in com.alibaba.csp.sentinel.cluster.server.codec.data.PingRequestDataDecoder.decode to see the decoded oversized length

Principle

According to the nmap port characteristic rule base, DNSVersionBindReqTCP type probe packets are misdecoded by the token server as ping packets.
https://raw.githubusercontent.com/nmap/nmap/refs/heads/master/nmap-service-probes

Does this pull request fix one issue?

Fixes #3636

This fixes excessive allocation, out-of-bounds reads, and residual bytes affecting later decoding when malformed frames contain invalid length or count fields.

Describe how you did it

1. Added independent limits for the outer Netty frame, ParamFlow parameter count, and parameter string length.
2. Validated Ping lengths before allocation and required them to exactly match the bytes remaining in the current frame.
3. Validated ParamFlow counts, types, field lengths, and remaining bytes before every read, and rejected trailing bytes.
4. Protocol decoding failures are handled by exceptionCaught, which closes the connection.

Describe how to verify it

Added and executed 43 related tests covering:

1. Valid and empty Ping/ParamFlow requests.
2. Negative, oversized, mismatched, and truncated length/count fields.
3. Unknown parameter types, trailing bytes, empty payloads, and oversized outer frames.
4. Connection closure for protocol errors and oversized frames.

Special notes for reviews

This fix addresses a potential security and performance issue where malformed packets could cause excessive memory allocation. The solution introduces proper validation and limits on packet sizes to prevent denial-of-service scenarios.

[CLAassistant]

All committers have signed the CLA.

[yungyu16]

这个问题，有可能被恶意利用，影响微服务进程可用性。

[yungyu16]

为什么我已经签署了CLA，但是机器人没有更新状态

[LilMosey]

为什么我已经签署了CLA，但是机器人没有更新状态

我刚刚提交也是这样的，和你一样的提示。
yungyu16 seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
本地控制台执行
git log -1 --pretty=format:"%an <%ae>"
然后到你的github主页添加一下你的邮箱就好了～

[yungyu16]

@uuuyuqi

[uuuyuqi]

Thanks for working on this. Validating network-provided lengths before allocating memory is the correct direction. However, the current implementation still needs the following changes:

1. readableBytes is captured before source.readInt(), so it includes the 4-byte length field, while packetLen only represents the following payload. These values cannot be compared directly; otherwise, every valid Ping request will be rejected. For example, when the namespace is "default", readableBytes is 11 while packetLen is 7.

   The comparison should be performed after reading the length field:

   int packetLen = source.readInt();
   int actualLength = source.readableBytes();
   if (packetLen != actualLength) {
       // discard the malformed frame
   }

   The Nmap frame declares a length of 0x01000000 but has only 21 bytes remaining, so it should be rejected before allocating the byte array.

2. When validation fails, the entire current frame should be discarded or the connection should be closed. Otherwise, unread bytes may remain in NettyRequestDecoder and interfere with subsequent frames. There is no need to emit a warning for every malformed frame at the protocol layer. Silently dropping the frame or closing the connection is sufficient; if observability is needed, a counter or rate-limited log would be more appropriate.

3. Validation in ParamFlowRequestDataDecoder is still incomplete. Changing the initial ArrayList capacity from the externally supplied amount to a fixed value is reasonable, but amount itself still needs a reasonable upper bound. A string length must also be non-negative and no greater than source.readableBytes().

4. NETTY_MAX_FRAME_LENGTH and the maximum length of an individual string field are separate protocol constraints. The former limits the entire transport frame, while the latter limits one application-level field, so they should not share the same constant. Otherwise, changing the outer frame limit would unintentionally change the allowed string size. If an independent string-length limit is required, it should have a dedicated constant. Regardless of that limit, the decoder must always validate the number of bytes actually remaining in the current frame.

5. Please add tests covering a valid Ping request, the current Nmap probe, negative lengths, declared lengths greater than the remaining bytes, truncated frames, and invalid ParamFlow amount and string lengths.

The Nmap service probe happens to match the current Sentinel protocol layout. A protocol magic header could prevent this kind of accidental protocol misidentification, but that should be treated as a separate protocol-evolution topic and does not replace validation of length and count fields.

The PR history also contains a merge commit used to synchronize with the upstream branch. Please rebase/squash it before merging and link the corresponding issue as required by the project contribution guidelines.

[yungyu16]

@uuuyuqi Thanks for the review. I have addressed the feedback, added the corresponding issue (#3636), completed the decoder and connection-handling tests, and confirmed that the branch is based on the latest upstream 1.8. Could you please take another look when you have time?

[yungyu16]

@LearningGp This PR addresses malformed frame length/count validation in the cluster token server, includes regression tests, links issue #3636, and is based on the latest 1.8 branch. When convenient, could you please review it and merge it if everything looks good? Thanks!

[uuuyuqi]

Thanks for the update. The current commit correctly addresses the original memory-allocation issue: Ping now compares the declared length with the actual remaining bytes before allocating the byte array, so the Nmap frame no longer causes a 16 MiB allocation. The ParamFlow validation for parameter counts, string lengths, negative values, and trailing bytes has also been completed.

However, complete outer-frame handling is still incomplete. LengthFieldBasedFrameDecoder already delivers a complete frame to NettyRequestDecoder, so each outer frame should either produce one complete request or be rejected as malformed. The current code still has the following cases:

1. When the request header is shorter than 5 bytes, DefaultRequestEntityDecoder returns null and the bytes remain in NettyRequestDecoder. If a valid Ping is sent afterward, the retained bytes are combined with the Ping and the valid Ping fails.

2. For an unknown request type, the decoder consumes xid/type and returns null. The remaining payload may be interpreted as another request during the same decode pass, or retained and combined with a later frame.

3. FlowRequestDataDecoder returns null for a truncated payload of 1–11 bytes. DefaultRequestEntityDecoder then still creates a Flow request with data == null. With the default processor, this request causes a NullPointerException in FlowRequestProcessor instead of a clear protocol exception. The truncated bytes may also be interpreted again as other requests.

4. A normal Flow payload is 12 or 13 bytes, but a payload of 14 bytes or more is still accepted and the extra bytes are not rejected. In verification, one trailing byte remained and corrupted a subsequent valid Ping.

5. A complete frame containing only xid + type=PING, with the Ping length field entirely missing, bypasses PingRequestDataDecoder. This happens because DefaultRequestEntityDecoder sets data = null without invoking the registered data decoder when no payload bytes remain. TokenServerHandler then returns a BAD response and keeps the connection open. The existing testDecodeEmptyNamespace covers a present 4-byte length field whose value is zero; it does not cover a completely missing length field.

For the current Nmap frame, the first length check closes the connection, so the original large allocation is prevented. However, 21 bytes remain unread when the exception is thrown. When closing the connection triggers channelInactive, Netty's ByteToMessageDecoder processes its remaining internal data again; in verification, this produces another CorruptedFrameException during shutdown.

I verified these behaviors with EmbeddedChannel using the actual handler order. I suggest ensuring the following behavior consistently:

• Invoke the request-specific data decoder even when zero payload bytes remain, so required fields are validated.
• Treat request == null as a malformed complete frame.
• After successful request decoding, treat any unread bytes in the current frame as malformed.
• Discard the remaining bytes of the current frame before propagating a protocol exception and closing the connection.
• Make FlowRequestDataDecoder explicitly reject both truncated and trailing data.

Please add EmbeddedChannel tests using the actual processing order:

LengthFieldBasedFrameDecoder
-> NettyRequestDecoder
-> TokenServerHandler

At minimum, the tests should cover a request header shorter than 5 bytes, an unknown request type, a Ping with the length field completely missing, a truncated Flow request, a Flow request with trailing bytes, the current Nmap frame, and a malformed frame followed by a valid Ping. The current tests mainly invoke leaf decoders directly or manually fire exceptionCaught, so they do not verify these behaviors.

These issues do not change the conclusion that the original 16 MiB allocation problem has been fixed, but the current implementation does not yet fully satisfy the requirement that malformed frames leave no residual data that can affect subsequent requests.