Skip to content

fix(arrow-cast): make b64_encode reject invalid UTF-8 from misbehaving Engine impls #10324

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

Merged
Jefffrey merged 6 commits into from
Jul 19, 2026
+54 −5
Merged

fix(arrow-cast): make b64_encode reject invalid UTF-8 from misbehaving Engine impls #10324

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
ad7b169
fix(arrow-cast): make b64_encode reject invalid UTF-8 from misbehavin…
bit2swaz Jul 11, 2026
a1185fc
Merge branch 'main' into fix/b64-encode-utf8-soundness
bit2swaz Jul 12, 2026
bad0cd2
Merge branch 'main' into fix/b64-encode-utf8-soundness
bit2swaz Jul 15, 2026
de05de8
Merge branch 'main' into fix/b64-encode-utf8-soundness
bit2swaz Jul 16, 2026
a83db3b
Merge branch 'main' into fix/b64-encode-utf8-soundness
bit2swaz Jul 17, 2026
1e22d00
fix: validate b64_encode output is UTF-8 to avoid unsound StringArray
bit2swaz Jul 17, 2026
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
59 changes: 54 additions & 5 deletions arrow-cast/src/base64.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
Expand All @@ -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())

@Rich-T-kid Rich-T-kid Jul 12, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it makes sense to have either an alternate function or passing in a boolean for utf8 validation.
I don't think its should be arrow-rs responsibility to validate the engine by default, especially when it comes with performance penalty.

maybe something similar to

pub fn b64_encode<E: Engine, O: OffsetSizeTrait>(
    engine: &E,
    array: &GenericBinaryArray<O>,
    validate_utf8: boolean,
)

and

switch validate_utf8 {
true  => GenericStringArray::try_new(offsets, Buffer::from_vec(buffer), array.nulls().cloned())
false => GenericStringArray::new_unchecked(offsets, Buffer::from_vec(buffer), array.nulls().cloned())
}

to avoid introducing an breaking API change it may make sense to make a separate function that handles the validation.

pub fn b64_encode_validate<E: Engine, O: OffsetSizeTrait>(
    engine: &E,
    array: &GenericBinaryArray<O>,
)

.expect("Engine produced invalid UTF-8")
}

/// Base64 decode each element of `array` with the provided [`Engine`]
Expand Down Expand Up @@ -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);
}
}
Loading