bit2swaz commented on issue #10284:
URL: https://github.com/apache/arrow-rs/issues/10284#issuecomment-4948177968

   the `// Safety: Base64 is valid UTF-8` comment in 
[`b64_encode`](https://github.com/apache/arrow-rs/blob/970391f2481be274707395ba3f7828618355da5b/arrow-cast/src/base64.rs#L52-L55)
 only holds for a correct `Engine`. but `base64::Engine` is a safe trait so a 
safe-rust impl can write non-ascii into the buffer and the `new_unchecked` 
`StringArray` then hands out a `&str` over invalid utf-8. thats UB reachable 
from 100% safe code
   
   reproduced it independently against `arrow-cast` 56 with a safe `EvilEngine` 
(no `unsafe` in the repro): the resulting `StringArray` backs onto `[ff ff 
...]`, `str::from_utf8` rejects those bytes, and reading `value(0)` aborts 
under miri at `char::from_u32_unchecked` (`0x1fffff`, not a valid scalar)
   
   to be clear on scope: this is a soundness hole, not a remotely exploitable 
bug, the trigger is a caller-supplied engine, not untrusted input
   
   the clean fix is to stop trusting the engine's output: build the array 
through the checked constructor instead of `new_unchecked`. `b64_decode` right 
below already does this at 
[base64.rs#L82](https://github.com/apache/arrow-rs/blob/970391f2481be274707395ba3f7828618355da5b/arrow-cast/src/base64.rs#L82)
 with `GenericBinaryArray::try_new(...) -> Result`, and 
`GenericStringType::validate` runs `std::str::from_utf8` on the values so a 
misbehaving engine surfaces as an `Err` rather than UB
   
   that does mean changing the signature to `-> Result<GenericStringArray<O>, 
ArrowError>`, matching `b64_decode`. the only in-tree caller is the arrow-json 
doc example (uses `BASE64_STANDARD`), which just needs a `?`. the alternative 
is marking `b64_encode` as `unsafe fn` and documenting the engine precondition 
but that pushes `unsafe` onto every honest caller to guard against a contrived 
one so id lean toward the checked-constructor route
   
   happy to take this. got any preference for the shape before i open the PR?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to