You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "Arawoof06 (via GitHub)" <gi...@apache.org> on 2026/08/18 12:11:20 UTC
[PR] GH-1261: Reject out-of-range dictionary indices in decode [arrow-java]
Arawoof06 opened a new pull request, #1262:
URL: https://github.com/apache/arrow-java/pull/1262
DictionaryEncoder.retrieveIndexVector guards each index with `indexAsInt > dictionaryCount`, but valid indices run 0..dictionaryCount-1, so an index equal to the count reads one slot past the dictionary vector and a negative index from a signed index type is not caught at all; both reach copyValueSafe. The index vector is decoded straight from an IPC payload, so a crafted dictionary-encoded batch reads out of bounds of the dictionary buffers when arrow.enable_unsafe_memory_access is set. Tightening the bound to reject negative indices and indices past the count also covers the list and struct sub-field decoders, which go through the same helper.
Closes #1261.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-1261: Reject out-of-range dictionary indices in decode [arrow-java]
Posted by "laurentgo (via GitHub)" <gi...@apache.org>.
laurentgo commented on PR #1262:
URL: https://github.com/apache/arrow-java/pull/1262#issuecomment-6122359042
Closed as requested by author
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-1261: Reject out-of-range dictionary indices in decode [arrow-java]
Posted by "laurentgo (via GitHub)" <gi...@apache.org>.
laurentgo closed pull request #1262: GH-1261: Reject out-of-range dictionary indices in decode
URL: https://github.com/apache/arrow-java/pull/1262
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-1261: Reject out-of-range dictionary indices in decode [arrow-java]
Posted by "Arawoof06 (via GitHub)" <gi...@apache.org>.
Arawoof06 commented on PR #1262:
URL: https://github.com/apache/arrow-java/pull/1262#issuecomment-6044059053
Both points are right, and both are on main now. #1309 landed the range check on the `long` before the narrowing cast, and switched `StructSubfieldEncoder` to pass `dictionary.getVector().getValueCount()` instead of the struct row count. That is what you asked for here, including the 64-bit wrap case I had missed, and my description was wrong to claim the struct path fell out of the one-line change.
I checked the three `retrieveIndexVector` callers on main and they all pass the dictionary's own value count now, so this branch has nothing left to contribute. Fine to close it as superseded by #1309.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-1261: Reject out-of-range dictionary indices in decode [arrow-java]
Posted by "jbonofre (via GitHub)" <gi...@apache.org>.
jbonofre commented on code in PR #1262:
URL: https://github.com/apache/arrow-java/pull/1262#discussion_r4186941500
##########
vector/src/main/java/org/apache/arrow/vector/dictionary/DictionaryEncoder.java:
##########
@@ -165,7 +165,7 @@ static void retrieveIndexVector(
for (int i = start; i < end; i++) {
if (!indices.isNull(i)) {
int indexAsInt = (int) indices.getValueAsLong(i);
- if (indexAsInt > dictionaryCount) {
+ if (indexAsInt < 0 || indexAsInt >= dictionaryCount) {
Review Comment:
This helper has a caller that doesn't pass a dictionary size. `StructSubfieldEncoder.decode` calls:
```java
DictionaryEncoder.retrieveIndexVector(indices, transfer, valueCount, 0, valueCount);
```
where `valueCount` is the struct's row count. With `>=` that gives two problems:
- A 1-row struct whose `f0` child holds index 1 into a 2-entry dictionary `["aa", "bb"]` decodes on main and now throws `Provided dictionary does not contain value for index 1`.
- A 40-row struct with index 39 into a 1-entry dictionary passes `39 >= 40` and reaches `copyValueSafe`. For a fixed-width dictionary that ends in a raw `MemoryUtil.copyMemory` in `BaseFixedWidthVector.copyFrom`.
Could you pass `dictionary.getVector().getValueCount()` in `StructSubfieldEncoder` instead? The PR description says the struct decoder is covered by this change, which only holds once that bound is fixed.
##########
vector/src/main/java/org/apache/arrow/vector/dictionary/DictionaryEncoder.java:
##########
@@ -165,7 +165,7 @@ static void retrieveIndexVector(
for (int i = start; i < end; i++) {
if (!indices.isNull(i)) {
int indexAsInt = (int) indices.getValueAsLong(i);
- if (indexAsInt > dictionaryCount) {
+ if (indexAsInt < 0 || indexAsInt >= dictionaryCount) {
throw new IllegalArgumentException(
"Provided dictionary does not contain value for index " + indexAsInt);
}
Review Comment:
The narrowing to `int` happens before the range check, so a 64-bit index can wrap into range. With a `BigIntVector` index, `4294967296` becomes `0` and `-4294967295` becomes `1`, and both are accepted. A `UInt4` index of `4294967295` is rejected, but the message reports `index -1`.
Checking the `long` first fixes both:
```suggestion
long index = indices.getValueAsLong(i);
if (index < 0 || index >= dictionaryCount) {
throw new IllegalArgumentException(
"Provided dictionary does not contain value for index " + index);
}
int indexAsInt = (int) index;
```
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-1261: Reject out-of-range dictionary indices in decode [arrow-java]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #1262:
URL: https://github.com/apache/arrow-java/pull/1262#issuecomment-5328567190
<!-- label_helper_comment -->
Thank you for opening a pull request!
Please label the PR with one or more of:
- bug-fix
- chore
- dependencies
- documentation
- enhancement
Also, add the 'breaking-change' label if appropriate.
See [CONTRIBUTING.md](https://github.com/apache/arrow-java/blob/main/CONTRIBUTING.md) for details.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org