This is an automated email from the ASF dual-hosted git repository.
Jefffrey pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-rs.git
The following commit(s) were added to refs/heads/main by this push:
new cc83f7eb25 fix(arrow-cast): make `b64_encode` reject invalid UTF-8
from misbehaving `Engine` impls (#10324)
cc83f7eb25 is described below
commit cc83f7eb25fbf0f6e8f94656d8fd703470e32c04
Author: Aditya Mishra <[email protected]>
AuthorDate: Sun Jul 19 07:51:13 2026 +0530
fix(arrow-cast): make `b64_encode` reject invalid UTF-8 from misbehaving
`Engine` impls (#10324)
## Which issue does this PR close?
Fixes #10284
## Rationale for this change
`b64_encode` builds a `GenericStringArray` from the output of a
caller-supplied `base64::Engine` via `new_unchecked` guarded by a `//
Safety: Base64 is valid UTF-8` comment. That comment only holds for a
*correct* engine. `base64::Engine` is a safe trait so a caller can
implement it in 100% safe Rust to write non-UTF-8 bytes into the buffer.
The resulting `StringArray` then hands out an invalid `&str` which is UB
reached from entirely safe code
A safe fn has to be sound for all safe inputs so this is a soundness
hole. It is not a remotely exploitable vuln: the trigger is a
caller-supplied `Engine`, not untrusted data
## What changes are included in this PR?
- `b64_encode` now returns `Result<GenericStringArray<O>, ArrowError>`
and builds the array through the checked `GenericStringArray::try_new`
constructor instead of `unsafe { new_unchecked(...) }`. A misbehaving
engine now surfaces as an `Err` rather than UB. This matches
`b64_decode`, which already returns `Result`
- The `unsafe` block is gone
- Updated the one in-tree caller (the `arrow-json` doc example)
- Added a regression test with a safe-Rust `EvilEngine` (the issue's
repro) that asserts `b64_encode` returns `Err`
`b64_decode` is untouched. It returns `GenericBinaryArray`, which has no
UTF-8 invariant, so this bug does not apply to it
## Verification
- `cargo test -p arrow-cast -p arrow-json` passes.
- Checked under Miri both ways: the `EvilEngine` repro aborts with UB
(`char::from_u32_unchecked`) against the current `new_unchecked` code,
and returns `Err` with no UB after this fix.
## Are there any user-facing changes?
Yes, this is a breaking API change. `b64_encode` now returns `Result`,
so callers have to handle the error (with `?` or `.unwrap()`).
There is an alternative: mark `b64_encode` as `unsafe fn` and document
the engine precondition. That has zero runtime cost but pushes `unsafe`
onto every honest caller. I went with the `Result` route to keep the
safe API safe, but I'm happy to switch to the `unsafe fn` shape if
maintainers prefer it.
Credit to the reporter (@Manishearth) for the Miri repro
---
arrow-cast/src/base64.rs | 59 ++++++++++++++++++++++++++++++++++++++++++++----
1 file changed, 54 insertions(+), 5 deletions(-)
diff --git a/arrow-cast/src/base64.rs b/arrow-cast/src/base64.rs
index 6a8da0141d..ea53aa5dcc 100644
--- a/arrow-cast/src/base64.rs
+++ b/arrow-cast/src/base64.rs
@@ -27,7 +27,11 @@ use base64::engine::Config;
pub use base64::prelude::*;
-/// Bas64 encode each element of `array` with the provided [`Engine`]
+/// Base64 encode each element of `array` with the provided [`Engine`]
+///
+/// Panics if the `Engine` emits output that is not valid UTF-8. A correct
+/// `Engine` never does, but it is a safe trait so a misbehaving impl could;
+/// validating keeps the returned [`GenericStringArray`] sound (#10284).
pub fn b64_encode<E: Engine, O: OffsetSizeTrait>(
engine: &E,
array: &GenericBinaryArray<O>,
@@ -49,10 +53,9 @@ pub fn b64_encode<E: Engine, O: OffsetSizeTrait>(
}
assert_eq!(offset, buffer_len);
- // Safety: Base64 is valid UTF-8
- unsafe {
- GenericStringArray::new_unchecked(offsets, Buffer::from_vec(buffer),
array.nulls().cloned())
- }
+ // `try_new` validates UTF-8 instead of trusting the (safe-trait) Engine.
+ GenericStringArray::try_new(offsets, Buffer::from_vec(buffer),
array.nulls().cloned())
+ .expect("Engine produced invalid UTF-8")
}
/// Base64 decode each element of `array` with the provided [`Engine`]
@@ -113,4 +116,50 @@ mod tests {
test_engine(&BASE64_STANDARD, &data);
test_engine(&BASE64_STANDARD_NO_PAD, &data);
}
+
+ /// Safe-Rust `Engine` that writes invalid UTF-8 into the encode buffer
+ /// (#10284). `b64_encode` must reject it rather than build an unsound
+ /// `StringArray`.
+ struct EvilEngine;
+
+ impl Engine for EvilEngine {
+ type Config = <base64::engine::GeneralPurpose as Engine>::Config;
+ type DecodeEstimate = <base64::engine::GeneralPurpose as
Engine>::DecodeEstimate;
+
+ fn internal_encode(&self, input: &[u8], output: &mut [u8]) -> usize {
+ BASE64_STANDARD.internal_encode(input, output)
+ }
+ fn internal_decoded_len_estimate(&self, input_len: usize) ->
Self::DecodeEstimate {
+ BASE64_STANDARD.internal_decoded_len_estimate(input_len)
+ }
+ fn internal_decode(
+ &self,
+ input: &[u8],
+ output: &mut [u8],
+ estimate: Self::DecodeEstimate,
+ ) -> Result<base64::engine::DecodeMetadata, base64::DecodeSliceError> {
+ BASE64_STANDARD.internal_decode(input, output, estimate)
+ }
+ fn config(&self) -> &Self::Config {
+ BASE64_STANDARD.config()
+ }
+ fn encode_slice<T: AsRef<[u8]>>(
+ &self,
+ input: T,
+ output_buf: &mut [u8],
+ ) -> Result<usize, base64::EncodeSliceError> {
+ let len = BASE64_STANDARD.encode_slice(input, output_buf)?;
+ for b in output_buf[..len].iter_mut() {
+ *b = 0xFF; // invalid UTF-8, but correct length
+ }
+ Ok(len)
+ }
+ }
+
+ #[test]
+ #[should_panic(expected = "produced invalid UTF-8")]
+ fn test_b64_encode_rejects_invalid_utf8() {
+ let data: BinaryArray =
vec![Some(b"hello".to_vec())].into_iter().collect();
+ let _ = b64_encode(&EvilEngine, &data);
+ }
}
