MarkLee131 opened a new issue, #63603: URL: https://github.com/apache/doris/issues/63603
### Search before asking - [x] I had searched in the [issues](https://github.com/apache/doris/issues?q=is%3Aissue) and found no similar issues. ### Version master, verified on commit `440a6d3b57e` (also present on `4938d638e3f`). ### What's Wrong? `MysqlProto.readLenEncodedString` reads a length-encoded integer and passes it straight to `new byte[(int) length]` with no bound: ```java public static byte[] readLenEncodedString(ByteBuffer buffer) { long length = readVInt(buffer); byte[] buf = new byte[(int) length]; // no upper bound buffer.get(buf); return buf; } ``` `readVInt` returns a full 8-byte value when the lead byte is `0xFE`, so `length` is whatever the client sends. Two bad cases fall out of the `(int)` cast: - low 32 bits `0x7FFFFFFF` -> `new byte[2147483647]`, a ~2 GiB allocation from a handful of bytes on the wire. - high bit set (e.g. `0xFFFFFFFF`) -> `(int) -1` -> `new byte[-1]` -> `NegativeArraySizeException`. This runs during the handshake, before authentication: `AcceptListener` -> `MysqlProto.negotiate` -> `MysqlAuthPacket.readFrom`, which calls `readLenEncodedString` for the auth-response field (`MysqlAuthPacket.java:93`) and for every key/value in the connection-attributes loop (`MysqlAuthPacket.java:110-118`). To be precise about impact: a single packet is not a hard crash. `AcceptListener` catches `Throwable`, so the exception (or an `OutOfMemoryError` from the allocation) is caught and the connection is dropped. The concern is that an unauthenticated client controls the allocation size at all, turning ~40 bytes into a multi-GB allocation request and adding avoidable GC pressure under connection spam. It is also just a fragile pattern for a parser on the network edge. ### What You Expected? A length-encoded string whose declared length is negative or larger than the bytes remaining in the packet should be rejected as a protocol error, not passed to `new byte[]`. A well-formed length-encoded string's payload always fits in the remaining buffer, so bounding by `buffer.remaining()` does not affect valid input. ### How to Reproduce? Unit test (in the PR, `MysqlProtoLenEncStringTest`): feed `readLenEncodedString` a `0xFE` lead byte plus an 8-byte length of `0x7FFFFFFF` (or `0xFFFFFFFF`) with only a few bytes remaining. Before the fix the first allocates ~2 GiB and the second throws `NegativeArraySizeException`; after the fix both are rejected with `IllegalArgumentException`, and a normal short length-encoded string still parses. ### Anything Else? PR ready: bounds `length` by `buffer.remaining()` before allocating, with a unit test. One guard covers both reach paths (auth-response and connection-attributes). ### Are you willing to submit PR? - [x] Yes I am willing to submit a PR! ### Code of Conduct - [x] I agree to follow this project's [Code of Conduct](https://www.apache.org/foundation/policies/conduct) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
