[
https://issues.apache.org/jira/browse/IMPALA-15221?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18100737#comment-18100737
]
ASF subversion and git services commented on IMPALA-15221:
----------------------------------------------------------
Commit 7fe7e1b35a11e5db9f6bdad636bb4159fa08ee16 in impala's branch
refs/heads/master from Zoltan Borok-Nagy
[ https://gitbox.apache.org/repos/asf?p=impala.git;h=7fe7e1b35 ]
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]>
Reviewed-on: http://gerrit.cloudera.org:8080/24635
Tested-by: Impala Public Jenkins <[email protected]>
Reviewed-by: Peter Rozsa <[email protected]>
> Harden VARIANT decoding against corrupt/malicious offsets
> ---------------------------------------------------------
>
> Key: IMPALA-15221
> URL: https://issues.apache.org/jira/browse/IMPALA-15221
> Project: IMPALA
> Issue Type: Bug
> Components: Backend
> Reporter: Zoltán Borók-Nagy
> Assignee: Zoltán Borók-Nagy
> Priority: Major
> Labels: impala-iceberg, impala-iceberg-active-backlog
>
> The variant decoder in be/src/runtime/variant-value.cc performs almost no
> bounds checking on offsets and lengths read from the value/metadata blobs.
> Since VARIANT is an external interchange type written by other engines
> (Spark, Flink, Trino, DuckDB) into Iceberg/Parquet files, these blobs are
> untrusted input. A corrupt or adversarial
> blob can cause reads outside the allocated buffer — a crash or information
> disclosure (unrelated heap memory streamed into query results via JSON
> conversion), not just an incorrect result.
> Unchecked paths include:
> - VariantMetadata::Init() — offset values are never validated to point
> within the string-data region, and (dict_size_ + 1) * offset_size_ is
> computed as int, so a large dict_size_ overflows and defeats the length guard.
> - GetString/GetBinary — string/binary length is taken directly from the
> blob with no check against the buffer length.
> - GetFieldByIndex/GetArrayElement — element offsets are unchecked and
> next_offset - field_offset can unsigned-underflow.
> - Object/array field counts drive raw pointer arithmetic with no bound.
> - The only existing guards are DCHECKs, which are compiled out in release
> builds.
> This was surfaced in review of IMPALA-15052 (gerrit 24521), which makes the
> decoder reachable from client-facing query output.
> Fix:
> - Metadata: validate the offset table once in Init() (monotonic, last
> offset ≤ string-data length) and fix the integer overflow.
> - Value data: add per-access bounds guards (string/binary length, element
> offsets, field counts) and convert ReadValue's DCHECK to a real check.
> Testing: add a negative-test matrix in variant-util-test.cc covering
> truncated metadata, out-of-range/non-monotonic offsets, overflowing
> dict_size, oversized string/binary lengths, underflowing element lengths, and
> out-of-range field ids.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
