Suspicious lifetime of C++ exception messages

[norru]

I've just spotted the following pattern in many functions:

int CEXR_InputFile_from_stream(CEXR_IStream *stream, int threads, CEXR_InputFile **out, const char **err_out) {
    try {
        *out = reinterpret_cast<CEXR_InputFile *>(new InputFile(*reinterpret_cast<IStream *>(stream), threads));
    } catch(const std::exception &e) {
        *err_out = e.what();
        return 1;
    }

    return 0;
}

At the call site:

        let mut error_out = ptr::null();
        let error = unsafe { CEXR_InputFile_from_stream(istream_ptr, 1, &mut out, &mut error_out) };
        if error != 0 {
            let msg = unsafe { CStr::from_ptr(error_out) };
            Err(Error::Generic(msg.to_string_lossy().into_owned()))
        } else {
...

It looks highly suspicious as the string pointed by error_out at the point of use is outside its "owner's" lifetime (the exception object).

The original exception object should be destroyed at the exit of the catch block, making the pointer coming out of it a dangling reference.

I wonder how this Could Have Possibly Ever Worked™. Am I missing something? I hope I am wrong here because, otherwise, the security implications would be huge.

[Ralith]

I think this is indeed a serious bug, and has worked historically the same way use-after-free usually does; the memory just hadn't been clobbered yet. Each instance of this pattern should be replaced with copying the exception string into a rust-allocated buffer.

[norru]

Bummer. Worked because of luck then. A Russian roulette bug.

I agree on the solution strategy.

[Ralith]

A convenient approach might be to have rust code define a constructor function returning a pointer from Vec::into_raw and callable from C inside the exception handler, and then pass the resulting value right back to rust where the value can be recovered with from_raw.

e: This would make linking more fragile, and we don't need to care that much about performance in the error case, so I just went with an extra malloc.