Skip to content

Commit 94dd987

Browse files
sfc-gh-hkaraucursoragentholdenk
committed
[CORE][MINOR] Validate array counts against received bytes in shuffle fetch decoders
FetchShuffleBlocks and FetchShuffleBlockChunks decode an outer element count and eagerly allocate a 2D array of that size. Check the count against the bytes actually received before allocating, mirroring the bounds checks Encoders already applies: each inner group carries its own 4-byte length prefix, so a valid count never exceeds readableBytes() / 4. Co-authored-by: Cursor <cursoragent@cursor.com> Co-Authored-By: Holden Karau <holden@pigscanfly.ca> (cherry picked from commit 9687793)
1 parent c0a9086 commit 94dd987

4 files changed

Lines changed: 37 additions & 0 deletions

File tree

common/network-shuffle/src/main/java/org/apache/spark/network/shuffle/protocol/FetchShuffleBlockChunks.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
package org.apache.spark.network.shuffle.protocol;
1919

2020
import java.util.Arrays;
21+
import java.util.Objects;
2122

2223
import io.netty.buffer.ByteBuf;
2324

@@ -130,6 +131,9 @@ public static FetchShuffleBlockChunks decode(ByteBuf buf) {
130131
int shuffleMergeId = buf.readInt();
131132
int[] reduceIds = Encoders.IntArrays.decode(buf);
132133
int chunkIdsLen = buf.readInt();
134+
// The divisor 4 is the minimum on-wire size of one element, since each chunk-id group
135+
// is prefixed with its own 4-byte length.
136+
Objects.checkFromIndexSize(0, chunkIdsLen, buf.readableBytes() / 4);
133137
int[][] chunkIds = new int[chunkIdsLen][];
134138
for (int i = 0; i < chunkIdsLen; i++) {
135139
chunkIds[i] = Encoders.IntArrays.decode(buf);

common/network-shuffle/src/main/java/org/apache/spark/network/shuffle/protocol/FetchShuffleBlocks.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
package org.apache.spark.network.shuffle.protocol;
1919

2020
import java.util.Arrays;
21+
import java.util.Objects;
2122

2223
import io.netty.buffer.ByteBuf;
2324

@@ -130,6 +131,9 @@ public static FetchShuffleBlocks decode(ByteBuf buf) {
130131
int shuffleId = buf.readInt();
131132
long[] mapIds = Encoders.LongArrays.decode(buf);
132133
int reduceIdsSize = buf.readInt();
134+
// The divisor 4 is the minimum on-wire size of one element, since each reduce-id group
135+
// is prefixed with its own 4-byte length.
136+
Objects.checkFromIndexSize(0, reduceIdsSize, buf.readableBytes() / 4);
133137
int[][] reduceIds = new int[reduceIdsSize][];
134138
for (int i = 0; i < reduceIdsSize; i++) {
135139
reduceIds[i] = Encoders.IntArrays.decode(buf);

common/network-shuffle/src/test/java/org/apache/spark/network/shuffle/protocol/FetchShuffleBlockChunksSuite.java

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@
2424

2525
import static org.junit.jupiter.api.Assertions.*;
2626

27+
import org.apache.spark.network.protocol.Encoders;
28+
2729
public class FetchShuffleBlockChunksSuite {
2830

2931
@Test
@@ -39,4 +41,17 @@ public void testFetchShuffleBlockChunksEncodeDecode() {
3941
FetchShuffleBlockChunks decoded = FetchShuffleBlockChunks.decode(buf);
4042
assertEquals(shuffleBlockChunks, decoded);
4143
}
44+
45+
@Test
46+
public void testDecodeRejectsCountExceedingReceivedBytes() {
47+
// Decode must reject the count rather than allocate an array of the claimed size.
48+
ByteBuf buf = Unpooled.buffer(64);
49+
Encoders.Strings.encode(buf, "app0");
50+
Encoders.Strings.encode(buf, "exec1");
51+
buf.writeInt(0); // shuffleId
52+
buf.writeInt(0); // shuffleMergeId
53+
Encoders.IntArrays.encode(buf, new int[] {0});
54+
buf.writeInt(Integer.MAX_VALUE); // chunkIds count
55+
assertThrows(IndexOutOfBoundsException.class, () -> FetchShuffleBlockChunks.decode(buf));
56+
}
4257
}

common/network-shuffle/src/test/java/org/apache/spark/network/shuffle/protocol/FetchShuffleBlocksSuite.java

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@
2424

2525
import static org.junit.jupiter.api.Assertions.*;
2626

27+
import org.apache.spark.network.protocol.Encoders;
28+
2729
public class FetchShuffleBlocksSuite {
2830

2931
@Test
@@ -39,4 +41,16 @@ public void testFetchShuffleBlockEncodeDecode() {
3941
FetchShuffleBlocks decoded = FetchShuffleBlocks.decode(buf);
4042
assertEquals(fetchShuffleBlocks, decoded);
4143
}
44+
45+
@Test
46+
public void testDecodeRejectsCountExceedingReceivedBytes() {
47+
// Decode must reject the count rather than allocate an array of the claimed size.
48+
ByteBuf buf = Unpooled.buffer(64);
49+
Encoders.Strings.encode(buf, "app0");
50+
Encoders.Strings.encode(buf, "exec1");
51+
buf.writeInt(0); // shuffleId
52+
Encoders.LongArrays.encode(buf, new long[] {0});
53+
buf.writeInt(Integer.MAX_VALUE); // reduceIds count
54+
assertThrows(IndexOutOfBoundsException.class, () -> FetchShuffleBlocks.decode(buf));
55+
}
4256
}

0 commit comments

Comments
 (0)