CString/.as_ptr() is incredibly unsafe :(

1

I knew that CString::new().as_ptr() is dangerous, so I consciously avoided that convenient construct, and still, I got a use-after-free.

I needed a nullable string for a C function, so I wrote:

let option = Some("foo");

// Explicitly bind CString to a variable to keep it alive
let c_option = option.and_then(|s| CString::new(s).ok()); 

// And it's not alive! This causes use-after-free, and Rust is doesn't say a word about it
let p = c_option.map(|c| c.as_ptr()).unwrap_or(ptr::null());

// And this is (I guess?) correct code:
let p = c_option.as_ref().map(|c| c.as_ptr()).unwrap_or(ptr::null());

So apparently Option::map moves the CString and frees it immediately after it gives a (dangling) pointer.

That's really, really bad. Even though the pointer isn't dereferenced on Rust's side so Rust can claim it's not unsafe, it is a real safety problem for Rust programs.

Can something be done about it?

  • e.g. return &c_char, so that the lifetime is tracked, and then rely on implicit conversion to *const c_char.
  • return a newtype, like KeepAlive<'a>(*const c_char) with Deref/AsRef
  • special magic in the compiler to give a warning or extend the lifetime
2

This seems like a variation on the "as_ptr is tricky to use" issue? Namely, you're still using as_ptr.

3

I guess, although I'm having unusually bad luck with CString.

It might be that Vec tends to have permanent lifetime, because it's useful in Rust, rather than a temporary object. Also I need to pass .as_ptr() and .len() together, which generally forces a more long-lived binding.

4

clippy has a lint against the vanilla CString::as_ptr misuse - maybe it can be extended to catch this scenario too? I don't know how hard/easy that is there, nor if adding this variation is worthwhile. But, clippy does know about this booby trap :slight_smile:.

5

Perhaps a method that takes a callback would be appropriate:

cstr.with_ptr(|ptr| call_ffi_func(ptr, data))
6

That would be safer, but quite inconvenient for more than one string.

Could Rust be extended to optionally track lifetime of raw pointers?

fn as_ptr<'a>(&'a self) -> *'a const c_str

The difference being that a raw pointer with lifetime can have its lifetime erased by assignment to *const char (*'static c_char).

7

This is more of a "pick your poison" approach, but you could use CString::into_raw to take ownership of the allocation, and then free it manually (via some RAII helper perhaps) with from_raw. as_ptr does seem like a foot gun though.

8

This seems like a good idea to have for FFI purposes.

9

There was an RFC to this effect: https://github.com/rust-lang/rfcs/pull/1642

10

That used to be the API.

Can't find the change where they went way from it, though.

11

Once we get truly unsized types (opaque data structures), &CStr could have the same representation as *const c_char (i.e., be a thin pointer to a null terminated string) and FFI functions could take &CStr instead of *const c_char†. One would then be able to simply call my_c_func(&my_cstring).

†Assuming we can rely on the representation of &OpaqueData being the same as *const OpaqueData.

12

Maybe better than "always bind the CString to a local variable" is "always explicitly drop the CString yourself". So if the last line of your function was

drop(c_option);

then the compiler would be able to give you an error here, because it knows the value has already dropped. It's a nice way to make your intention super clear. With Option involved, you'd have to be careful not to Option::take the string out, which would make its lifetime independent of the Option you're dropping. Maybe something like this would be nice:

if let Some(my_cstring) = c_option {
    // do some pointer stuff
    drop(my_cstring);
}
13

Why not just drop the Option itself?

14

The example at the top is probably a simplified version of some code where the Option is doing something more important?

15

Yes, indeed. In this case I'm parsing command line arguments, and some arguments are truly optional, so Option seems right for this purpose. Then I'm passing the arguments to C via a struct, where optionality is expressed as NULL.

For a second I thought returning &c_char would be great, but I also need NULL :frowning: It won't work with .unwrap_or(ptr::null())

17

We tried to change as_ptr back to with_ptr, but that makes it hard to use if you are dealing with multiple strings, and convenience won over footgun regulation there. Instead we got the destructor hack which zeroes out the string on drop, so at least you find the problem relatively quickly (because it never "happens to work").