Hello Peter Rozsa, Csaba Ringhofer, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/24635
to look at the new patch set (#7).
Change subject: IMPALA-15221: Harden VARIANT decoding against corrupt input
......................................................................
IMPALA-15221: Harden VARIANT decoding against corrupt input
The VariantValue/VariantMetadata decoder added in IMPALA-15067 performed
almost no bounds checking on the offsets and lengths read from the value
and metadata blobs. Because VARIANT values can be written by other
engines into Iceberg/Parquet files, these blobs are untrusted input: a
corrupt or adversarial blob could cause reads outside the allocated
buffer: a crash, or information disclosure via the JSON conversion
This patch adds bounds checking throughout the decoder.
Metadata (validated once in VariantMetadata::Init):
- Compute the offset-array length in 64-bit so a corrupt (large)
dictionary size cannot overflow the length check.
- Record the string-data length and verify the dictionary offsets are
non-decreasing and stay within it, so GetFieldName() can never return
a string_view that points out of bounds.
Value (checked on access):
- Scalar accessors (GetInt*, GetFloat/GetDouble, GetString/GetBinary and
the ReadValue<T> helper) now return a bool and fail on a type mismatch
or a truncated payload instead of reading past the end of the buffer.
- Object/array navigation (GetFieldByIndex, GetArrayElement,
GetFieldByName, GetObjectSize, GetArraySize, GetFieldNameByIndex)
parses and bounds-checks the header and offset table before use,
validates each element's extent, and checks that field ids are within
the metadata dictionary.
- JSON serialization enforces a maximum nesting depth so a deeply nested
value cannot overflow the stack, and returns an error on any malformed
encoding.
Checking bounds at each access, keeps partial-access callers such as
variant_get() lazy and lets them share the same protection.
Testing:
- Added negative unit tests in variant-util-test.cc
- Existing tests pass
Change-Id: I3a22bb16cd05854a9afb56ebd27806ac52670bca
Assisted-by: Claude Opus 4.8 (1M context) <[email protected]>
---
M be/src/exprs/variant-functions-ir.cc
M be/src/runtime/types.h
M be/src/runtime/variant-value.cc
M be/src/runtime/variant-value.h
M be/src/util/variant-util-test.cc
M fe/src/main/java/org/apache/impala/catalog/Type.java
6 files changed, 588 insertions(+), 270 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/35/24635/7
--
To view, visit http://gerrit.cloudera.org:8080/24635
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3a22bb16cd05854a9afb56ebd27806ac52670bca
Gerrit-Change-Number: 24635
Gerrit-PatchSet: 7
Gerrit-Owner: Zoltan Borok-Nagy <[email protected]>
Gerrit-Reviewer: Csaba Ringhofer <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Peter Rozsa <[email protected]>
Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]>
