Skip to content

fix: Don't read offsets from a too-short offsets buffer in default validation #987

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

Open
pedrobtz wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: Don't read offsets from a too-short offsets buffer in default validation #987

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 16 additions & 4 deletions src/nanoarrow/common/array.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1834,7 +1834,10 @@ static int ArrowArrayViewValidateDefault(struct ArrowArrayView* array_view,
switch (array_view->storage_type) {
case NANOARROW_TYPE_STRING:
case NANOARROW_TYPE_BINARY:
if (array_view->buffer_views[1].size_bytes != 0) {
// A zero-length array may have an offsets buffer that is too short to
// hold one offset: only read offsets from a buffer that holds them
if (array_view->buffer_views[1].size_bytes >=
(offset_plus_length + 1) * (int64_t)sizeof(int32_t)) {
first_offset = ArrowBufferViewGetInt32Unsafe(&array_view->buffer_views[1],
array_view->offset);
if (first_offset < 0) {
Expand Down Expand Up @@ -1872,7 +1875,10 @@ static int ArrowArrayViewValidateDefault(struct ArrowArrayView* array_view,

case NANOARROW_TYPE_LARGE_STRING:
case NANOARROW_TYPE_LARGE_BINARY:
if (array_view->buffer_views[1].size_bytes != 0) {
// A zero-length array may have an offsets buffer that is too short to
// hold one offset: only read offsets from a buffer that holds them
if (array_view->buffer_views[1].size_bytes >=
(offset_plus_length + 1) * (int64_t)sizeof(int64_t)) {
first_offset = ArrowBufferViewGetInt64Unsafe(&array_view->buffer_views[1],
array_view->offset);
if (first_offset < 0) {
Expand Down Expand Up @@ -1923,7 +1929,10 @@ static int ArrowArrayViewValidateDefault(struct ArrowArrayView* array_view,

case NANOARROW_TYPE_LIST:
case NANOARROW_TYPE_MAP:
if (array_view->buffer_views[1].size_bytes != 0) {
// A zero-length array may have an offsets buffer that is too short to
// hold one offset: only read offsets from a buffer that holds them
if (array_view->buffer_views[1].size_bytes >=
(offset_plus_length + 1) * (int64_t)sizeof(int32_t)) {
first_offset = ArrowBufferViewGetInt32Unsafe(&array_view->buffer_views[1],
array_view->offset);
if (first_offset < 0) {
Expand Down Expand Up @@ -1953,7 +1962,10 @@ static int ArrowArrayViewValidateDefault(struct ArrowArrayView* array_view,
break;

case NANOARROW_TYPE_LARGE_LIST:
if (array_view->buffer_views[1].size_bytes != 0) {
// A zero-length array may have an offsets buffer that is too short to
// hold one offset: only read offsets from a buffer that holds them
if (array_view->buffer_views[1].size_bytes >=
(offset_plus_length + 1) * (int64_t)sizeof(int64_t)) {
first_offset = ArrowBufferViewGetInt64Unsafe(&array_view->buffer_views[1],
array_view->offset);
if (first_offset < 0) {
Expand Down
33 changes: 33 additions & 0 deletions src/nanoarrow/common/array_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include <cmath>
#include <cstdint>
#include <cstring>
#include <memory>
#include <thread>

#if defined(NANOARROW_BUILD_TESTS_WITH_ARROW)
Expand Down Expand Up @@ -2839,6 +2840,38 @@ TEST(ArrayTest, ArrayViewTestMove) {
ArrowArrayViewReset(&array_view2);
}

TEST(ArrayTest, ArrayViewTestShortOffsetsBuffer) {
// A zero-length array passes minimal validation with an offsets buffer that is
// too short to hold one offset; default validation must not read an offset from it
struct ArrowError error;
for (enum ArrowType type : {NANOARROW_TYPE_STRING, NANOARROW_TYPE_LARGE_STRING,
NANOARROW_TYPE_LIST, NANOARROW_TYPE_LARGE_LIST}) {
nanoarrow::UniqueSchema schema;
ASSERT_EQ(ArrowSchemaInitFromType(schema.get(), type), NANOARROW_OK);
if (type == NANOARROW_TYPE_LIST || type == NANOARROW_TYPE_LARGE_LIST) {
ASSERT_EQ(ArrowSchemaSetType(schema->children[0], NANOARROW_TYPE_INT32),
NANOARROW_OK);
}

nanoarrow::UniqueArrayView array_view;
ASSERT_EQ(ArrowArrayViewInitFromSchema(array_view.get(), schema.get(), &error),
NANOARROW_OK);
ArrowArrayViewSetLength(array_view.get(), 0);

// One byte short of a single offset, in a heap buffer of exactly that size
int64_t offset_bytes =
ArrowArrayViewGetBufferElementSizeBits(array_view.get(), 1) / 8;
std::unique_ptr<uint8_t[]> offsets(new uint8_t[offset_bytes - 1]());
array_view->buffer_views[1].data.as_uint8 = offsets.get();
array_view->buffer_views[1].size_bytes = offset_bytes - 1;

EXPECT_EQ(ArrowArrayViewValidate(array_view.get(), NANOARROW_VALIDATION_LEVEL_DEFAULT,
&error),
NANOARROW_OK)
<< ArrowTypeString(type) << ": " << error.message;
}
}

TEST(ArrayTest, ArrayViewTestString) {
struct ArrowArrayView array_view;
struct ArrowError error;
Expand Down