search

mozilla / audioipc

Cubeb Audio Remoting For Gecko
10 stars 17 forks source link

Avoid using the pointer of a temporary variable #94

Closed ChunMinChang closed 4 years ago

ChunMinChang commented 4 years ago

The std::ffi::CString::new(id).unwrap() will create a temporary variable. std::ffi::CString::new(id).unwrap().as_ptr() is a pointer pointing to a temporary variable. Using this pointer directly is unsafe since we cannot make sure the variable is alive when the pointer is referred. It's better to save the variable returned from CString::new() and use its pointer (in the same scope of the variable) when we need.

Run cargo clippy with CString::new(text).unwrap().as_ptr() will lead to a temporary_cstring_as_ptr error. Here is a playground test.

r? @kinetiknz

kinetiknz commented 4 years ago

Thanks for filing! CString's API is easy to misuse, I've run into similar issues before too. I'll make a note to set clippy up in CI when I get a chance to catch this type of issue quicker.

Interestingly, in this particular case I believe clippy's warning is incorrect. The lifetime of the CString extends over the duration of the function call (see https://doc.rust-lang.org/reference/expressions.html#temporary-lifetimes), so there is no UAF in this instance. See this playground, where running MIRI from the Tools menu indicates the issue exists for baz() but not for bar(). It's also visible by selecting MIR from the Build menu and examining the lifetimes of the various local variables in the IR.