lucasbru commented on code in PR #22463:
URL: https://github.com/apache/kafka/pull/22463#discussion_r3354879327
##########
clients/src/main/java/org/apache/kafka/common/serialization/ListDeserializer.java:
##########
@@ -173,12 +176,19 @@ public List<Inner> deserialize(String topic, Headers
headers, byte[] data) {
List<Integer> nullIndexList = null;
if (serStrategy == SerializationStrategy.CONSTANT_SIZE) {
// In CONSTANT_SIZE strategy, indexes of null entries are
decoded from a null index list
- nullIndexList = deserializeNullIndexList(dis);
+ nullIndexList = deserializeNullIndexList(dis, data.length);
}
final int size = dis.readInt();
+ if (size > data.length) {
+ throw new SerializationException("Corrupted byte[]. The number
of list entries cannot be larger than overall number of bytes.");
+ }
List<Inner> deserializedList = createListInstance(size);
for (int i = 0; i < size; i++) {
int entrySize = serStrategy ==
SerializationStrategy.CONSTANT_SIZE ? primitiveSize : dis.readInt();
+ if (entrySize > data.length) {
Review Comment:
These guards catch oversized counts but not negative ones — readInt() can
return a negative entrySize (or size). A negative entrySize falls through to
`new byte[entrySize]` below and throws NegativeArraySizeException, and a
negative size throws from the list constructor; neither gets wrapped as
SerializationException. Worth rejecting `< 0` here too so corrupt input fails
consistently?
##########
clients/src/main/java/org/apache/kafka/common/serialization/ListDeserializer.java:
##########
@@ -148,8 +148,11 @@ private SerializationStrategy
parseSerializationStrategyFlag(final int serializa
return SerializationStrategy.VALUES[serializationStrategyFlag];
}
- private List<Integer> deserializeNullIndexList(final DataInputStream dis)
throws IOException {
+ private List<Integer> deserializeNullIndexList(final DataInputStream dis,
final int length) throws IOException {
int nullIndexListSize = dis.readInt();
+ if (nullIndexListSize > length) {
Review Comment:
Could we add a couple of tests that feed a corrupted byte[] (oversized
null-index size, oversized entry count, oversized entry size) and assert
SerializationException? These three new branches are otherwise uncovered.
--
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]
