Stream: wg-secure-code

Topic: vec_append_from_within


Shnatsel (Jun 20 2019 at 21:38, on Zulip):

This "append part of vector to itself" problem has caused bugs in two crates and even a CVE in stdlib. I'm writing an RFC for a stdlib abtraction to fix this once and for all. Sneak peek:

assert_eq!(vec![3,5,7].append_from_within((..1)), vec![3,5,7,3]);
assert_eq!(vec![3,5,7].append_from_within((1..)), vec![3,5,7,5,7]);
assert_eq!(vec![3,5,7].append_from_within((..)), vec![3,5,7,3,5,7]);
vec![3,5,7].append_from_within((..20)); // panic!
Shnatsel (Jun 20 2019 at 21:40, on Zulip):

Do I understand correctly that if you want to read several elements from the vector, modify them in some way and append them to the vector you'd have to copy them onto the stack or a different heap location? Otherwise modification would affect the elements within the vector, right?

Shnatsel (Jun 20 2019 at 21:48, on Zulip):

Oh yeah, important constraint: I also want to push them all at the same time

Tony Arcieri (Jun 20 2019 at 22:56, on Zulip):

that sounds like the safest way to avoid aliasing issues to me...

Tony Arcieri (Jun 20 2019 at 22:57, on Zulip):

so is the issue people are doing unsafe in-place modifications and running into aliasing bugs?

Shnatsel (Jun 20 2019 at 23:56, on Zulip):

The issue is people are trying to append large chunks of the vector to itself. There is no safe way to do that, so they hand-roll unsafe code that does set_len() on vector and then fill it in by copying from the already initialized part. RLE implementations end up not handling the degenerate case where the offset for copying is 0 and they just iterate over the buffer reading uninitialized bytes and writing them back in their place. Stdlib dodged that (hopefully - I haven't checked), but didn't handle the overflow in length computation and ended up with a buffer overflow bug, see CVE-2018-1000810

Shnatsel (Jun 21 2019 at 00:04, on Zulip):

The RFC is almost complete, here's what I've got so far: https://gist.github.com/Shnatsel/e3907b56921e1b6be187aad1d86efc3e

Alex Gaynor (Jun 21 2019 at 12:00, on Zulip):

FWIW your python example has slightly different semantics, since slicing a list performs a copy, so:

.extend method works even when passing a subslice of the same list
Is mostly irrelevant since subslices aren't a thing.

Shnatsel (Jun 21 2019 at 20:18, on Zulip):

Thanks, I've amended it to make that more clear. Also replaced the C example with a memcpy.

Shnatsel (Jun 21 2019 at 20:18, on Zulip):

RFC is now up for review: https://github.com/rust-lang/rfcs/pull/2714

Tony Arcieri (Jul 01 2019 at 17:07, on Zulip):

subscribed to the issue

Last update: Nov 11 2019 at 23:10UTC