Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[C++] ValidateRunEndEncoded does not check there is no null buffer #34696

Closed
wjones127 opened this issue Mar 22, 2023 · 0 comments · Fixed by #34697
Closed

[C++] ValidateRunEndEncoded does not check there is no null buffer #34696

wjones127 opened this issue Mar 22, 2023 · 0 comments · Fixed by #34697
Assignees
@wjones127
Labels
Component: C++ Critical Fix Bugfixes for security vulnerabilities, crashes, or invalid data. Type: bug
Milestone
12.0.0

Comments

@wjones127
Copy link
Member

Describe the bug, including details regarding any error messages, version, and platform.

We should make sure we check there is no null buffer at the top-level, otherwise we will attempt to count the nulls in GetNullCount, which can crash if the null bitmap is invalid. This situation can happen if corrupt data is read through an IPC reader.

Component(s)

C++
@wjones127 wjones127 self-assigned this Mar 22, 2023
@wjones127 wjones127 added Critical Fix Bugfixes for security vulnerabilities, crashes, or invalid data. and removed Component: C++ labels Mar 22, 2023
@wjones127 wjones127 mentioned this issue Mar 22, 2023
Merged
1 task
wjones127 added a commit that referenced this issue Mar 28, 2023
@wjones127
GH-34696: [C++] Check REE arrays have no null buffer in Validate() (#…
02bc24c 
…34697)

### Rationale for this change

See #34696

### What changes are included in this PR?

Adds another check. Also removes DCHECKs from `SetData()`, since they are redundant with the `Validate()` checks and I don't think we do that for other arrays. Having them present made it harder to get to the root cause of this error, because `SetData()` is called on the array when validating a record batch.

### Are these changes tested?

* [x] Add tests

### Are there any user-facing changes?

**This PR contains a "Critical Fix".** Without this, sending a REE array with an invalid null buffer through IPC to the C++ implementation will cause it to crash if the array is validated.
* Closes: #34696

Authored-by: Will Jones <willjones127@gmail.com>
Signed-off-by: Will Jones <willjones127@gmail.com>
@wjones127 wjones127 added this to the 12.0.0 milestone Mar 28, 2023
ArgusLi pushed a commit to Bit-Quill/arrow that referenced this issue May 15, 2023
@wjones127 @ArgusLi
apacheGH-34696: [C++] Check REE arrays have no null buffer in Validat…
c4b4410 
…e() (apache#34697)

### Rationale for this change

See apache#34696

### What changes are included in this PR?

Adds another check. Also removes DCHECKs from `SetData()`, since they are redundant with the `Validate()` checks and I don't think we do that for other arrays. Having them present made it harder to get to the root cause of this error, because `SetData()` is called on the array when validating a record batch.

### Are these changes tested?

* [x] Add tests

### Are there any user-facing changes?

**This PR contains a "Critical Fix".** Without this, sending a REE array with an invalid null buffer through IPC to the C++ implementation will cause it to crash if the array is validated.
* Closes: apache#34696

Authored-by: Will Jones <willjones127@gmail.com>
Signed-off-by: Will Jones <willjones127@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@wjones127 wjones127

Labels
Component: C++ Critical Fix Bugfixes for security vulnerabilities, crashes, or invalid data. Type: bug
Projects
None yet
Milestone
12.0.0
Development

Successfully merging a pull request may close this issue.

GH-34696: [C++] Check REE arrays have no null buffer in Validate() wjones127/arrow
1 participant
@wjones127