These are chat archives for rust-lang/rust

16th
Mar 2016
Pete Hayes
@petehayes102
Mar 16 2016 08:32

Hey guys. I’ve come across a buffer overflow issue with some FFI work I’m doing. If I create a string buffer with String::with_capacity(n), I get an overflow in my C code.

let buf = String::with_capacity(9);
let c_buf = CString::new(buf).unwrap().into_raw();

my_c_fn(c_buf, "my string");
// Boom!

But this works...

let buf = "000000000"; // 9 chars
let c_buf = CString::new(buf).unwrap().into_raw();

my_c_fn(c_buf, "my string");
// Hooray!

I'm confused!

Vladimir Matveev
@netvl
Mar 16 2016 08:33
@petehayes102 CString owns the buffer you pass to it
but after into_raw() call it is destroyed, because it is only a temporary object here
then the pointer into_raw() has returned starts pointing to garbage
try this:
let buf = String::with_capacity(9);
let c_buf = CString::new(buf).unwrap();

my_c_fn(c_buf, "my string")
hmm, that said, it may also be that CString::new() does something with the allocation as well
ahh, please disregard all of the above
Pete Hayes
@petehayes102
Mar 16 2016 08:35
Oh ok, that makes sense. Your code though won’t match the fn signature, as it’s expecting *c_char
Vladimir Matveev
@netvl
Mar 16 2016 08:35
into_raw() consumes the buffer, right
Pete Hayes
@petehayes102
Mar 16 2016 08:35
Ok :P
Vladimir Matveev
@netvl
Mar 16 2016 08:35
I confused it with as_ptr()
yes, my second thought seems to be correct
    pub fn new<T: Into<Vec<u8>>>(t: T) -> Result<CString, NulError> {
        Self::_new(t.into())
    }

    fn _new(bytes: Vec<u8>) -> Result<CString, NulError> {
        match memchr::memchr(0, &bytes) {
            Some(i) => Err(NulError(i, bytes)),
            None => Ok(unsafe { CString::from_vec_unchecked(bytes) }),
        }
    }

    pub unsafe fn from_vec_unchecked(mut v: Vec<u8>) -> CString {
        v.push(0);
        CString { inner: v.into_boxed_slice() }
    }
You see, the vector is converted into a boxed slice
String::with_capacity() returns a string with zero length and the specified capacity
so it is first converted into a Vec<u8> with zero length
then a zero is pushed onto it and it is converted into a boxed slice
converting to a boxed slice involves shrinking the allocation to the actual length, because slices do not hold the capacity which would be necessary to deallocate them
thus the actual allocation which you pass to the C side is not 9 bytes in length
Pete Hayes
@petehayes102
Mar 16 2016 08:40
Ok, so by passing an actual string the vector’s minimum length is the length of the string, rather than zero as is the case with with_capacity()?
Vladimir Matveev
@netvl
Mar 16 2016 08:40
yep, that’s correct
Pete Hayes
@petehayes102
Mar 16 2016 08:41
Hooray! I understand something!
Thanks mate. I really appreciate you interpreting that for me
Vladimir Matveev
@netvl
Mar 16 2016 08:41
note that it’s not «minimum length», it is actual length, the length of the meaningful data in an allocation
Pete Hayes
@petehayes102
Mar 16 2016 08:41
So the best way for me to mitigate this problem is to ‘zero-fill’ a string buffer as in my example, or is there something more elegant?
Yep…my poor terminology there
Vladimir Matveev
@netvl
Mar 16 2016 08:42
I’m pretty sure that zero-filling the buffer will do no good, because CString means a zero-terminated string
which may only contain a zero at the end
you can see from the code above that new() will return a NulError if the data contains a zero
what do you actually want to do, pass a buffer to the C function?
you don’t need a CString for that, it is only needed if you need to pass some concrete zero-terminated string
Pete Hayes
@petehayes102
Mar 16 2016 08:43
yep. the C function fills that buffer with goodness and then I unpackage it in Rust and continue processing
Vladimir Matveev
@netvl
Mar 16 2016 08:44
you can use just Vec
Pete Hayes
@petehayes102
Mar 16 2016 08:44
Ok, I’ll give that a go. Cheers mate.
Vladimir Matveev
@netvl
Mar 16 2016 08:45
let mut buf = vec![0u8; 9];
unsafe { my_c_fn(buf.as_mut_ptr(), "my string") }
something like this
Pete Hayes
@petehayes102
Mar 16 2016 08:45
:thumbsup: Ta.
Vladimir Matveev
@netvl
Mar 16 2016 08:46
as_mut_ptr() does not consume the buffer, so it remains available after the function call (unlike CString::into_raw() above)
Vec::with_capacity() would also be incorrect because while the allocation will contain the specified amount of bytes, you won’t be able to read them after the C function invocation because the length will still remain zero
Pete Hayes
@petehayes102
Mar 16 2016 08:48
Np. I just tried that in the project and it works a treat. You’re a genius! :)
Vladimir Matveev
@netvl
Mar 16 2016 08:48
you’re welcome)