Skip to content

Commit f769ad7

Browse files
committed
ORC-2199: Validate union tag against the number of children in UnionTreeReader
### What changes were proposed in this pull request? This PR aims to validate the union tag against the number of children in `UnionTreeReader`. Both the `nextVector` and `skipRows` paths now check that each tag read from the `DATA` stream is within `[0, fields.length)` and throw `FileFormatException` with a clear message, consistent with the C++ reader's `getCheckedUnionTag`. ### Why are the changes needed? The Java `UnionTreeReader` used the raw tag byte from the `DATA` stream without validation. For a corrupt or malicious file with a tag greater than or equal to the number of union children (or a negative sign-extended byte), `skipRows` fails with `ArrayIndexOutOfBoundsException` and `nextVector` propagates the invalid tag downstream. The C++ reader already rejects this via `getCheckedUnionTag` (`ParseError`), so this closes a C++/Java parity gap. ### How was this patch tested? Pass the CIs with a newly added `TestTreeReaderFactory`. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude Fable 5 Closes #2682 from dongjoon-hyun/ORC-2199. Authored-by: Dongjoon Hyun <dongjoon@apache.org> Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
1 parent 5f2855d commit f769ad7

2 files changed

Lines changed: 144 additions & 1 deletion

File tree

java/core/src/java/org/apache/orc/impl/TreeReaderFactory.java

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2741,6 +2741,13 @@ protected UnionTreeReader(int columnId, InStream present,
27412741
this.fields = childReaders;
27422742
}
27432743

2744+
private void checkUnionTag(int tag) throws FileFormatException {
2745+
if (tag < 0 || tag >= fields.length) {
2746+
throw new FileFormatException("Invalid union tag " + tag +
2747+
" for union with " + fields.length + " children");
2748+
}
2749+
}
2750+
27442751
@Override
27452752
public void seek(PositionProvider[] index, ReadPhase readPhase) throws IOException {
27462753
if (readPhase.contains(this.readerCategory)) {
@@ -2767,6 +2774,11 @@ public void nextVector(ColumnVector previousVector,
27672774
result.isRepeating = false;
27682775
tags.nextVector(result.noNulls ? null : result.isNull, result.tags,
27692776
batchSize);
2777+
for (int r = 0; r < batchSize; ++r) {
2778+
if (result.noNulls || !result.isNull[r]) {
2779+
checkUnionTag(result.tags[r]);
2780+
}
2781+
}
27702782
}
27712783
}
27722784

@@ -2808,7 +2820,9 @@ public void skipRows(long items, ReadPhase readPhase) throws IOException {
28082820
items = countNonNulls(items);
28092821
long[] counts = new long[fields.length];
28102822
for (int i = 0; i < items; ++i) {
2811-
counts[tags.next()] += 1;
2823+
int tag = tags.next();
2824+
checkUnionTag(tag);
2825+
counts[tag] += 1;
28122826
}
28132827
for (int i = 0; i < counts.length; ++i) {
28142828
if (TypeReader.shouldProcessChild(fields[i], readPhase)) {
Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing, software
13+
* distributed under the License is distributed on an "AS IS" BASIS,
14+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
* See the License for the specific language governing permissions and
16+
* limitations under the License.
17+
*/
18+
19+
package org.apache.orc.impl;
20+
21+
import org.apache.hadoop.hive.ql.exec.vector.LongColumnVector;
22+
import org.apache.hadoop.hive.ql.exec.vector.UnionColumnVector;
23+
import org.apache.orc.FileFormatException;
24+
import org.apache.orc.OrcProto;
25+
import org.apache.orc.impl.reader.tree.TypeReader;
26+
import org.apache.orc.impl.writer.StreamOptions;
27+
import org.junit.jupiter.api.Test;
28+
29+
import java.io.IOException;
30+
import java.nio.ByteBuffer;
31+
32+
import static org.junit.jupiter.api.Assertions.assertEquals;
33+
import static org.junit.jupiter.api.Assertions.assertThrows;
34+
35+
public class TestTreeReaderFactory {
36+
37+
private static final OrcProto.ColumnEncoding DIRECT =
38+
OrcProto.ColumnEncoding.newBuilder()
39+
.setKind(OrcProto.ColumnEncoding.Kind.DIRECT).build();
40+
41+
private static InStream toInStream(TestInStream.OutputCollector collect) {
42+
ByteBuffer inBuf = ByteBuffer.allocate(collect.buffer.size());
43+
collect.buffer.setByteBuffer(inBuf, 0, collect.buffer.size());
44+
inBuf.flip();
45+
return InStream.create("test", new BufferChunk(inBuf, 0), 0,
46+
inBuf.remaining());
47+
}
48+
49+
private static RunLengthByteReader createTagReader(byte... tags)
50+
throws IOException {
51+
TestInStream.OutputCollector collect = new TestInStream.OutputCollector();
52+
RunLengthByteWriter writer = new RunLengthByteWriter(
53+
new OutStream("test", new StreamOptions(100), collect));
54+
for (byte tag : tags) {
55+
writer.write(tag);
56+
}
57+
writer.flush();
58+
return new RunLengthByteReader(toInStream(collect));
59+
}
60+
61+
private static InStream createLongStream(long... values) throws IOException {
62+
TestInStream.OutputCollector collect = new TestInStream.OutputCollector();
63+
RunLengthIntegerWriter writer = new RunLengthIntegerWriter(
64+
new OutStream("test", new StreamOptions(100), collect), true);
65+
for (long value : values) {
66+
writer.write(value);
67+
}
68+
writer.flush();
69+
return toInStream(collect);
70+
}
71+
72+
@Test
73+
public void testUnionWithValidTags() throws Exception {
74+
TreeReaderFactory.Context context = new TreeReaderFactory.ReaderContext();
75+
TypeReader[] children = new TypeReader[]{
76+
new TreeReaderFactory.LongTreeReader(1, null,
77+
createLongStream(10, 30), DIRECT, context),
78+
new TreeReaderFactory.LongTreeReader(2, null,
79+
createLongStream(20), DIRECT, context)};
80+
TreeReaderFactory.UnionTreeReader reader =
81+
new TreeReaderFactory.UnionTreeReader(0, null, context, null, children);
82+
reader.tags = createTagReader((byte) 0, (byte) 1, (byte) 0);
83+
84+
UnionColumnVector batch = new UnionColumnVector(3,
85+
new LongColumnVector(3), new LongColumnVector(3));
86+
reader.nextVector(batch, null, 3, null, TypeReader.ReadPhase.ALL);
87+
88+
assertEquals(0, batch.tags[0]);
89+
assertEquals(1, batch.tags[1]);
90+
assertEquals(0, batch.tags[2]);
91+
assertEquals(10, ((LongColumnVector) batch.fields[0]).vector[0]);
92+
assertEquals(20, ((LongColumnVector) batch.fields[1]).vector[1]);
93+
assertEquals(30, ((LongColumnVector) batch.fields[0]).vector[2]);
94+
}
95+
96+
@Test
97+
public void testUnionWithInvalidTagInNextVector() throws Exception {
98+
TreeReaderFactory.Context context = new TreeReaderFactory.ReaderContext();
99+
TypeReader[] children = new TypeReader[]{
100+
new TreeReaderFactory.LongTreeReader(1, context),
101+
new TreeReaderFactory.LongTreeReader(2, context)};
102+
TreeReaderFactory.UnionTreeReader reader =
103+
new TreeReaderFactory.UnionTreeReader(0, null, context, null, children);
104+
reader.tags = createTagReader((byte) 0, (byte) 1, (byte) 5);
105+
106+
UnionColumnVector batch = new UnionColumnVector(3,
107+
new LongColumnVector(3), new LongColumnVector(3));
108+
FileFormatException e = assertThrows(FileFormatException.class, () ->
109+
reader.nextVector(batch, null, 3, null, TypeReader.ReadPhase.ALL));
110+
assertEquals("Invalid union tag 5 for union with 2 children",
111+
e.getMessage());
112+
}
113+
114+
@Test
115+
public void testUnionWithInvalidTagInSkipRows() throws Exception {
116+
TreeReaderFactory.Context context = new TreeReaderFactory.ReaderContext();
117+
TypeReader[] children = new TypeReader[]{
118+
new TreeReaderFactory.LongTreeReader(1, context),
119+
new TreeReaderFactory.LongTreeReader(2, context)};
120+
TreeReaderFactory.UnionTreeReader reader =
121+
new TreeReaderFactory.UnionTreeReader(0, null, context, null, children);
122+
reader.tags = createTagReader((byte) 0, (byte) 1, (byte) 0x80);
123+
124+
FileFormatException e = assertThrows(FileFormatException.class, () ->
125+
reader.skipRows(3, TypeReader.ReadPhase.ALL));
126+
assertEquals("Invalid union tag -128 for union with 2 children",
127+
e.getMessage());
128+
}
129+
}

0 commit comments

Comments
 (0)