Bug: PCWSTR is potentially unsound in safe code if used wrong

[MolotovCherry]

Which crate is this about?

windows

Crate version

0.43.0

Summary

It is possible to cause corruption issues by using PCWSTR and not taking careful note about lifetimes and when things get dropped.

It might also be said that since we have to use the produced PCWSTR in unsafe code anyways, it's not strictly technically "unsafe" in safe code. But regardless, whatever course is decided from this report, I think this is important to take note of for people since it is a very easy gotcha to fall into.

Edit: Got here from #973 , and found that one of the posted solution comments had exactly this bug, demonstrating just how easy it is to accidentally get wrong. I really do feel that a convenience function to make a PCWSTR would help mitigate such a gotcha like this one

Toolchain version/configuration

Default host: x86_64-pc-windows-msvc
rustup home: C:\Users\MyUser\.rustup

installed toolchains

stable-x86_64-pc-windows-msvc
nightly-x86_64-pc-windows-msvc (default)
1.63-x86_64-pc-windows-msvc
1.65.0-x86_64-pc-windows-msvc

active toolchain

nightly-x86_64-pc-windows-msvc (default)
rustc 1.67.0-nightly (e75aab045 2022-11-09)

Reproducible example

For example, in the following, using PCWSTR at all is unsound, because the HSTRING it a temporary that gets immediately dropped, rendering the contents of the PCWSTR invalid from the get-go
PCWSTR(HSTRING::from(s).as_ptr())

To fix it, the values must be explicitly assigned so that they will drop after the usage of PCWSTR.

let h_title = HSTRING::from(title);
let h_message = HSTRING::from(message);

let title = PCWSTR::from_raw(h_title.as_ptr());
let message = PCWSTR::from_raw(h_message.as_ptr());

unsafe {
    MessageBoxW(None, message, title, icon.into());
}

Of course, it is possible to create this issue in other use-cases as well. The main point being, if the lifetime of the string isn't at least as long as PCWSTR, we're in trouble

Crate manifest

[dependencies.windows]
version = "0.43.0"
features = [
    "Win32_UI_WindowsAndMessaging",
    "Win32_Foundation"
]

Expected behavior

No corruption.

Actual behavior

Pointer in PCWSTR is invalid

Additional comments

Not sure the best way to fix this, but it is definitely a gotcha that many people may not be aware of.

[riverar]

Related to #2082, #2136, and others.

[kennykerr]

As @riverar pointed out, this has already been improved to some extent. While PCWSTR makes it relatively easy to write incorrect code, it is not in itself unsafe from a Rust perspective and care should be taken to use such APIs correctly.