Skip to content

Commit 111cb6b

Browse files
dfa1claude
dfa1
and
claude
committed
test(reader,writer): negative-path tests for hardening guards; tidy nits
Add the malformed-input tests deferred in the hardening PR, plus two small cleanups in the touched main files. Tests: - Content-Range parsing: missing prefix/'-'/'/', non-numeric fields, reversed separators, empty fields (MalformedHttpResponseTest). - HTTP segmentSpec bounds: out-of-range remote footer surfaces as VortexException, mirroring the local-file path (HttpSegmentSpecBoundsSecurityTest). - DType-tree and array-node depth caps: 65536-deep nesting rejected as VortexException, not StackOverflowError (DTypeDepthBombSecurityTest, ArrayNodeDepthBombSecurityTest). - zstd decode guards: negative/oversized frame size (IoBounds.toIntSize) and oversized VarBin length prefix on both the contiguous and scattered paths (readVarBinLen) in ZstdEncodingEncoderTest. Nits: - FlatSegmentDecoder: import VortexException instead of inline FQN. - ZstdEncodingDecoder: note the VarBin second pass relies on the first pass's readVarBinLen bounds check; keep both passes in lockstep. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent adc445e commit 111cb6b

8 files changed

Lines changed: 425 additions & 2 deletions

File tree

‎reader/src/main/java/io/github/dfa1/vortex/reader/FlatSegmentDecoder.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import static io.github.dfa1.vortex.core.io.PTypeIO.LE_INT;
44

5+
import io.github.dfa1.vortex.core.error.VortexException;
56
import io.github.dfa1.vortex.core.model.DType;
67
import io.github.dfa1.vortex.core.io.IoBounds;
78
import io.github.dfa1.vortex.reader.array.Array;
@@ -29,7 +30,7 @@ public final class FlatSegmentDecoder {
2930
/// Hard cap on array-node recursion depth. The encoded array tree nests through child nodes
3031
/// (validity, patches, run-ends, dictionary codes/values, …); a crafted or self-referential
3132
/// FlatBuffer can drive [#convertArrayNode] into unbounded recursion and a [StackOverflowError]
32-
/// — an `Error`, so it would bypass the [io.github.dfa1.vortex.core.error.VortexException]
33+
/// — an `Error`, so it would bypass the [VortexException]
3334
/// contract. 64 is well past any real encoding's nesting.
3435
static final int MAX_ARRAY_TREE_DEPTH = 64;
3536

@@ -82,7 +83,7 @@ private static ArrayNode convertArrayNode(
8283
int depth
8384
) {
8485
if (depth > MAX_ARRAY_TREE_DEPTH) {
85-
throw new io.github.dfa1.vortex.core.error.VortexException(
86+
throw new VortexException(
8687
"array tree depth exceeds limit (" + MAX_ARRAY_TREE_DEPTH + ")");
8788
}
8889
String rawEncodingId = encodingSpecs.get(fbs.encoding());

‎reader/src/main/java/io/github/dfa1/vortex/reader/decode/ZstdEncodingDecoder.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,9 @@ private static VarBinArray buildScatteredVarBin(
133133
MemorySegment offsets = ctx.arena().allocate((rowCount + 1) * 4L, 4);
134134
offsets.setAtIndex(PTypeIO.LE_INT, 0, 0);
135135

136+
// Second pass reads the same positions the first pass already bounds-checked via
137+
// readVarBinLen, so a raw get/copy here cannot overrun; values is sized to the validated
138+
// total. Keep both passes in lockstep — any edit to the cursor advance must stay identical.
136139
long readPos = 0;
137140
long dataPos = 0;
138141
for (long i = 0; i < rowCount; i++) {
@@ -277,6 +280,9 @@ private static VarBinArray buildVarBin(DType dtype, long n, MemorySegment decomp
277280
MemorySegment offsets = ctx.arena().allocate((n + 1) * 4L, 4);
278281
offsets.setAtIndex(PTypeIO.LE_INT, 0, 0);
279282

283+
// Second pass reads the same positions the first pass already bounds-checked via
284+
// readVarBinLen, so a raw get/copy here cannot overrun; values is sized to the validated
285+
// total. Keep both passes in lockstep — any edit to the cursor advance must stay identical.
280286
pos = 0;
281287
long dataPos = 0;
282288
for (long i = 0; i < n; i++) {

‎reader/src/test/java/io/github/dfa1/vortex/reader/ArrayNodeDepthBombSecurityTest.java‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
package io.github.dfa1.vortex.reader;
2+
3+
import io.github.dfa1.vortex.core.error.VortexException;
4+
import io.github.dfa1.vortex.core.fbs.FbsArray;
5+
import io.github.dfa1.vortex.core.fbs.FbsArrayNode;
6+
import io.github.dfa1.vortex.core.fbs.FbsBuilder;
7+
import io.github.dfa1.vortex.core.io.PTypeIO;
8+
import io.github.dfa1.vortex.core.model.DType;
9+
import org.junit.jupiter.api.Test;
10+
11+
import java.lang.foreign.Arena;
12+
import java.lang.foreign.MemorySegment;
13+
import java.util.List;
14+
15+
import static org.assertj.core.api.Assertions.assertThatThrownBy;
16+
17+
/// Adversarial test for the encoded-array-tree recursion in
18+
/// [FlatSegmentDecoder]'s `convertArrayNode`.
19+
///
20+
/// The decoder walks the array node tree recursively (validity, patches, run-ends, dictionary
21+
/// codes/values, …). Without the [FlatSegmentDecoder#MAX_ARRAY_TREE_DEPTH] cap a crafted segment
22+
/// with thousands of nested children produces a [StackOverflowError] — an `Error` that escapes the
23+
/// "malformed input must surface as [VortexException]" contract (ADR 0003). This pins that contract.
24+
class ArrayNodeDepthBombSecurityTest {
25+
26+
private static final DType DTYPE = DType.I32;
27+
28+
private final FlatSegmentDecoder sut = new FlatSegmentDecoder(ReadRegistry.empty());
29+
30+
@Test
31+
void deeplyNestedArrayTree_throwsVortexException() {
32+
try (Arena arena = Arena.ofConfined()) {
33+
// Given — a flat segment whose FbsArray root nests 65536 levels of single-child nodes.
34+
// Real encodings nest only a handful of levels; 65536 reliably blows the JVM stack on
35+
// the recursive convertArrayNode walk if the depth cap is removed.
36+
byte[] fb = deeplyNestedArrayFlatBuffer(65536);
37+
MemorySegment seg = arena.allocate(fb.length + 4L);
38+
MemorySegment.copy(MemorySegment.ofArray(fb), 0, seg, 0, fb.length);
39+
seg.set(PTypeIO.LE_INT, fb.length, fb.length);
40+
41+
// When / Then — must surface as VortexException, not StackOverflowError
42+
assertThatThrownBy(() -> sut.decode(seg, List.of("vortex.flat"), DTYPE, 1, arena))
43+
.isInstanceOf(VortexException.class);
44+
}
45+
}
46+
47+
/// Builds a minimal `FbsArray` whose root node has `depth` levels of single-child nesting,
48+
/// each level a buffer-less node referencing encoding index 0.
49+
private static byte[] deeplyNestedArrayFlatBuffer(int depth) {
50+
FbsBuilder b = new FbsBuilder(depth * 32);
51+
// Leaf first; FlatBuffer requires children be finished before parents.
52+
int emptyChildren = FbsArrayNode.createChildrenVector(b, new int[0]);
53+
int emptyBuffers = FbsArrayNode.createBuffersVector(b, new int[0]);
54+
int current = FbsArrayNode.createFbsArrayNode(b, 0, 0, emptyChildren, emptyBuffers, 0);
55+
for (int i = 0; i < depth; i++) {
56+
int childV = FbsArrayNode.createChildrenVector(b, new int[]{current});
57+
int bufV = FbsArrayNode.createBuffersVector(b, new int[0]);
58+
current = FbsArrayNode.createFbsArrayNode(b, 0, 0, childV, bufV, 0);
59+
}
60+
FbsArray.startBuffersVector(b, 0);
61+
int buffers = b.endVector();
62+
int array = FbsArray.createFbsArray(b, current, buffers);
63+
FbsArray.finishFbsArrayBuffer(b, array);
64+
return b.sizedByteArray();
65+
}
66+
}

‎reader/src/test/java/io/github/dfa1/vortex/reader/DTypeDepthBombSecurityTest.java‎

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
package io.github.dfa1.vortex.reader;
2+
3+
import io.github.dfa1.vortex.core.error.VortexException;
4+
import io.github.dfa1.vortex.core.io.VortexFormat;
5+
import org.junit.jupiter.api.Test;
6+
import org.junit.jupiter.api.io.TempDir;
7+
8+
import java.io.OutputStream;
9+
import java.nio.ByteBuffer;
10+
import java.nio.ByteOrder;
11+
import java.nio.file.Files;
12+
import java.nio.file.Path;
13+
14+
import static io.github.dfa1.vortex.reader.MalformedFiles.buildDeeplyNestedListDtype;
15+
import static io.github.dfa1.vortex.reader.MalformedFiles.buildFlatLayout;
16+
import static io.github.dfa1.vortex.reader.MalformedFiles.buildFooter;
17+
import static io.github.dfa1.vortex.reader.MalformedFiles.buildPostscript;
18+
import static org.assertj.core.api.Assertions.assertThatThrownBy;
19+
20+
/// Adversarial test for the DType-tree recursion in [PostscriptParser]'s `convertDType`.
21+
///
22+
/// `convertDType` walks Struct fields, List/FixedSizeList element types, and Extension storage
23+
/// types recursively. Without the [PostscriptParser#MAX_DTYPE_DEPTH] cap a crafted file with
24+
/// thousands of nested `List` types produces a [StackOverflowError] during `VortexReader.open` —
25+
/// an `Error` that escapes the [VortexException] sanitization and leaks the reader's memory-mapped
26+
/// Arena (`open`'s `catch (Exception)` never runs `arena.close()`). This pins the contract: deeply
27+
/// nested dtypes must be rejected as [VortexException], never a [StackOverflowError].
28+
class DTypeDepthBombSecurityTest {
29+
30+
private static final ReadRegistry REGISTRY = ReadRegistry.empty();
31+
32+
@Test
33+
void deeplyNestedDtype_throwsVortexException(@TempDir Path tmp) throws Exception {
34+
// Given — a file whose DType nests 65536 levels of List. Real schemas nest a handful of
35+
// levels; 65536 reliably blows the JVM stack on the recursive convertDType walk.
36+
Path file = buildDeeplyNestedDtypeFile(tmp, 65536);
37+
38+
// When / Then — must surface as VortexException, not StackOverflowError
39+
assertThatThrownBy(() -> VortexReader.open(file, REGISTRY))
40+
.isInstanceOf(VortexException.class);
41+
}
42+
43+
private static Path buildDeeplyNestedDtypeFile(Path dir, int depth) throws Exception {
44+
byte[] body = new byte[8]; // unused placeholder
45+
ByteBuffer footerBuf = buildFooter(
46+
new String[]{"vortex.primitive"},
47+
new String[]{"vortex.flat"},
48+
new long[]{0L},
49+
new long[]{(long) body.length});
50+
ByteBuffer dtypeBuf = buildDeeplyNestedListDtype(depth);
51+
ByteBuffer layoutBuf = buildFlatLayout(0, 1L, 0);
52+
53+
long footerOff = body.length;
54+
long dtypeOff = footerOff + footerBuf.remaining();
55+
long layoutOff = dtypeOff + dtypeBuf.remaining();
56+
57+
ByteBuffer psBuf = buildPostscript(
58+
footerOff, footerBuf.remaining(),
59+
dtypeOff, dtypeBuf.remaining(),
60+
layoutOff, layoutBuf.remaining());
61+
62+
int psLen = psBuf.remaining();
63+
ByteBuffer trailer = ByteBuffer.allocate(8).order(ByteOrder.LITTLE_ENDIAN);
64+
trailer.putShort((short) 1);
65+
trailer.putShort((short) psLen);
66+
trailer.put(VortexFormat.MAGIC.asByteBuffer());
67+
trailer.flip();
68+
69+
Path file = dir.resolve("deep_dtype.vtx");
70+
try (OutputStream out = Files.newOutputStream(file)) {
71+
out.write(body);
72+
writeBuf(out, footerBuf);
73+
writeBuf(out, dtypeBuf);
74+
writeBuf(out, layoutBuf);
75+
writeBuf(out, psBuf);
76+
out.write(trailer.array());
77+
}
78+
return file;
79+
}
80+
81+
private static void writeBuf(OutputStream out, ByteBuffer buf) throws Exception {
82+
buf = buf.duplicate();
83+
byte[] bytes = new byte[buf.remaining()];
84+
buf.get(bytes);
85+
out.write(bytes);
86+
}
87+
}

‎reader/src/test/java/io/github/dfa1/vortex/reader/HttpSegmentSpecBoundsSecurityTest.java‎

Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,148 @@
1+
package io.github.dfa1.vortex.reader;
2+
3+
import io.github.dfa1.vortex.core.error.VortexException;
4+
import io.github.dfa1.vortex.core.io.VortexFormat;
5+
import org.junit.jupiter.api.Test;
6+
import org.junit.jupiter.api.extension.ExtendWith;
7+
import org.mockito.Mock;
8+
import org.mockito.junit.jupiter.MockitoExtension;
9+
10+
import java.io.ByteArrayOutputStream;
11+
import java.net.URI;
12+
import java.net.http.HttpClient;
13+
import java.net.http.HttpHeaders;
14+
import java.net.http.HttpRequest;
15+
import java.net.http.HttpResponse;
16+
import java.nio.ByteBuffer;
17+
import java.nio.ByteOrder;
18+
import java.util.List;
19+
import java.util.Map;
20+
import java.util.Optional;
21+
import javax.net.ssl.SSLSession;
22+
23+
import static io.github.dfa1.vortex.reader.MalformedFiles.buildFlatLayout;
24+
import static io.github.dfa1.vortex.reader.MalformedFiles.buildFooter;
25+
import static io.github.dfa1.vortex.reader.MalformedFiles.buildI64Dtype;
26+
import static io.github.dfa1.vortex.reader.MalformedFiles.buildPostscript;
27+
import static org.assertj.core.api.Assertions.assertThatThrownBy;
28+
import static org.mockito.ArgumentMatchers.any;
29+
import static org.mockito.Mockito.doReturn;
30+
31+
/// Pins that the HTTP reader validates footer `segmentSpecs` against the file size, mirroring the
32+
/// local-file path. `VortexHttpReader.open` calls `PostscriptParser.parseBlobs` directly (not
33+
/// `parse`), so it must run `validateSegmentSpecs` itself — otherwise an out-of-bounds remote
34+
/// footer turns into out-of-range HTTP Range requests at scan time instead of a [VortexException].
35+
@ExtendWith(MockitoExtension.class)
36+
class HttpSegmentSpecBoundsSecurityTest {
37+
38+
@Mock
39+
private HttpClient client;
40+
41+
private static final URI URI = java.net.URI.create("http://example.com/oob_segment.vortex");
42+
43+
@Test
44+
void open_segmentSpecPastEof_throwsVortexException() throws Exception {
45+
// Given — a well-formed remote file whose single footer segmentSpec declares an offset far
46+
// beyond the file, served as a 206 that fits the tail window.
47+
byte[] file = buildFileWithOobSegment();
48+
doReturn(response206("bytes 0-" + (file.length - 1) + "/" + file.length, file))
49+
.when(client).send(any(), any());
50+
51+
// When / Then
52+
assertThatThrownBy(() -> VortexHttpReader.open(URI, ReadRegistry.empty(), client))
53+
.isInstanceOf(VortexException.class)
54+
.hasMessageContaining("out of bounds");
55+
}
56+
57+
private static byte[] buildFileWithOobSegment() throws Exception {
58+
byte[] body = new byte[8]; // unused placeholder
59+
// Segment offset 2^40 sits far past any real file size, so validateSegmentSpecs must reject.
60+
ByteBuffer footerBuf = buildFooter(
61+
new String[]{"vortex.primitive"},
62+
new String[]{"vortex.flat"},
63+
new long[]{1L << 40},
64+
new long[]{0L});
65+
ByteBuffer dtypeBuf = buildI64Dtype();
66+
ByteBuffer layoutBuf = buildFlatLayout(0, 1L, 0);
67+
68+
long footerOff = body.length;
69+
long dtypeOff = footerOff + footerBuf.remaining();
70+
long layoutOff = dtypeOff + dtypeBuf.remaining();
71+
72+
ByteBuffer psBuf = buildPostscript(
73+
footerOff, footerBuf.remaining(),
74+
dtypeOff, dtypeBuf.remaining(),
75+
layoutOff, layoutBuf.remaining());
76+
77+
int psLen = psBuf.remaining();
78+
ByteBuffer trailer = ByteBuffer.allocate(8).order(ByteOrder.LITTLE_ENDIAN);
79+
trailer.putShort((short) 1);
80+
trailer.putShort((short) psLen);
81+
trailer.put(VortexFormat.MAGIC.asByteBuffer());
82+
trailer.flip();
83+
84+
ByteArrayOutputStream out = new ByteArrayOutputStream();
85+
out.write(body);
86+
writeBuf(out, footerBuf);
87+
writeBuf(out, dtypeBuf);
88+
writeBuf(out, layoutBuf);
89+
writeBuf(out, psBuf);
90+
out.write(trailer.array());
91+
return out.toByteArray();
92+
}
93+
94+
private static void writeBuf(ByteArrayOutputStream out, ByteBuffer buf) {
95+
buf = buf.duplicate();
96+
byte[] bytes = new byte[buf.remaining()];
97+
buf.get(bytes);
98+
out.write(bytes, 0, bytes.length);
99+
}
100+
101+
@SuppressWarnings("unchecked")
102+
private static HttpResponse<byte[]> response206(String contentRange, byte[] body) {
103+
return new HttpResponse<>() {
104+
@Override
105+
public int statusCode() {
106+
return 206;
107+
}
108+
109+
@Override
110+
public byte[] body() {
111+
return body;
112+
}
113+
114+
@Override
115+
public HttpHeaders headers() {
116+
Map<String, List<String>> map = contentRange == null
117+
? Map.of()
118+
: Map.of("content-range", List.of(contentRange));
119+
return HttpHeaders.of(map, (k, v) -> true);
120+
}
121+
122+
@Override
123+
public HttpRequest request() {
124+
return null;
125+
}
126+
127+
@Override
128+
public Optional<HttpResponse<byte[]>> previousResponse() {
129+
return Optional.empty();
130+
}
131+
132+
@Override
133+
public Optional<SSLSession> sslSession() {
134+
return Optional.empty();
135+
}
136+
137+
@Override
138+
public java.net.URI uri() {
139+
return URI;
140+
}
141+
142+
@Override
143+
public HttpClient.Version version() {
144+
return HttpClient.Version.HTTP_1_1;
145+
}
146+
};
147+
}
148+
}

‎reader/src/test/java/io/github/dfa1/vortex/reader/MalformedFiles.java‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import io.github.dfa1.vortex.core.fbs.FbsFooter;
66
import io.github.dfa1.vortex.core.fbs.FbsLayout;
77
import io.github.dfa1.vortex.core.fbs.FbsLayoutSpec;
8+
import io.github.dfa1.vortex.core.fbs.FbsList;
89
import io.github.dfa1.vortex.core.fbs.FbsPostscript;
910
import io.github.dfa1.vortex.core.fbs.FbsPostscriptSegment;
1011
import io.github.dfa1.vortex.core.fbs.FbsPrimitive;
@@ -34,6 +35,23 @@ static ByteBuffer buildI64Dtype() {
3435
return slice(fbb);
3536
}
3637

38+
/// Builds a DType blob nesting `depth` levels of `List` around an I64 primitive leaf.
39+
///
40+
/// @param depth number of nested `List` wrappers around the leaf
41+
/// @return the finished DType FlatBuffer
42+
static ByteBuffer buildDeeplyNestedListDtype(int depth) {
43+
var fbb = new FbsBuilder(depth * 32);
44+
// Leaf first; FlatBuffer requires children be finished before parents.
45+
int prim = FbsPrimitive.createFbsPrimitive(fbb, io.github.dfa1.vortex.core.fbs.FbsPType.I64, false);
46+
int current = io.github.dfa1.vortex.core.fbs.FbsDType.createFbsDType(fbb, FbsType.FbsPrimitive, prim);
47+
for (int i = 0; i < depth; i++) {
48+
int list = FbsList.createFbsList(fbb, current, false);
49+
current = io.github.dfa1.vortex.core.fbs.FbsDType.createFbsDType(fbb, FbsType.FbsList, list);
50+
}
51+
io.github.dfa1.vortex.core.fbs.FbsDType.finishFbsDTypeBuffer(fbb, current);
52+
return slice(fbb);
53+
}
54+
3755
/// Builds a FbsFooter with the given array/layout spec ids and inline segment specs.
3856
///
3957
/// @param arraySpecs encoding ids, one per array spec

0 commit comments

Comments
 (0)