rust-lang/rust

API soundness issue in join() implementation of [Borrow<str>]

Qwaz opened this issue · 2 comments

Qwaz commented

A weird Borrow implementation that returns a different result for each call can create a string with uninitialized bytes with join() implementation of [Borrow<str>] type.

The problem is in join_generic_copy function.

  1. The borrow result is first used for the length calculation.

    // compute the exact total length of the joined Vec
    // if the `len` calculation overflows, we'll panic
    // we would have run out of memory anyway and the rest of the function requires
    // the entire Vec pre-allocated for safety
    let len = sep_len
    .checked_mul(iter.len())
    .and_then(|n| {
    slice.iter().map(|s| s.borrow().as_ref().len()).try_fold(n, usize::checked_add)
    })
    .expect("attempt to join into collection with len > usize::MAX");

  2. Then, inside spezialize_for_lengths macro, the user-provided slice is borrowed again and the content is copied.

    // arbitrary non-zero size fallback
    for s in iter {
    copy_slice_and_advance!(target, sep_bytes);
    copy_slice_and_advance!(target, s.borrow().as_ref());
    }

  3. Finally, the length of the slice is set to the length calculated in step 1.

    result.set_len(len);

Playground link, which demonstrates creating a non-UTF-8 string by only using safe Rust.

I guess all that’s needed is an assert!(target.is_empty()) at the end of the body of the spezialize_for_lengths macro? Perhaps the typo in its name could be fixed, too. It might also be beneficial to avoid the unnecessary third(!) call to borrow() that’s due to copy_slice_and_advance using its second argument twice.