Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Undefined behavior in provided method uWrite::write_char #30

Open
jrvanwhy opened this issue Mar 3, 2021 · 2 comments
Open

Undefined behavior in provided method uWrite::write_char #30

jrvanwhy opened this issue Mar 3, 2021 · 2 comments

Comments

@jrvanwhy
Copy link

jrvanwhy commented Mar 3, 2021

uWrite::write_char contains an unsound use of core::mem::uninitialized (see the docs for std::mem::MaybeUninit):

    fn write_char(&mut self, c: char) -> Result<(), Self::Error> {
        let mut buf: [u8; 4] = unsafe { uninitialized() };
        self.write_str(c.encode_utf8(&mut buf))
    }

The usual fix is to use std::mem::MaybeUninit, but this crate has a minimum supported Rust version of 1.34, which predates the stabilization of std::mem::MaybeUninit in 1.36.

There are a few possible fixes:

  1. Zero-initialize the array, either accepting extra generated code size or hoping the compiler optimizes it away.
  2. Increase the minimum supported Rust version to 1.36 and use std::mem::MaybeUninit.
  3. Re-implement std::mem::MaybeUninit in this crate (i.e. use a union to make it sound), which involves significantly more unsafe code.
@jrvanwhy
Copy link
Author

jrvanwhy commented Mar 3, 2021

Actually, looking again, encode_utf8 takes a &mut [u8], so it requires its input to be initialized. Therefore I believe solutions 2 and 3 from my post are unsound.

An additional option to avoid unnecessary overhead is to implement our own char-to-utf8 logic.

@jrvanwhy
Copy link
Author

jrvanwhy commented Mar 3, 2021

Actually, grepping through the source shows that uninitialized is used in several more places, which complicates the fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant