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

bool and char's Bytes implementations cause undefined behavior. #100

Open
zachs18 opened this issue May 25, 2022 · 0 comments · May be fixed by #101
Open

bool and char's Bytes implementations cause undefined behavior. #100

zachs18 opened this issue May 25, 2022 · 0 comments · May be fixed by #101

Comments

@zachs18
Copy link

zachs18 commented May 25, 2022

The documentation for secrets::traits::Bytes states:

Any type that implements Bytes must not exhibit undefined behavior when its underlying bits are set to any arbitrary bit pattern.

Currently, bool and char (the primitive types) have implementations for Bytes (https://github.com/stouset/secrets/blob/master/src/traits.rs#L69 ), but these types can not be set to arbitrary bit patterns (specifically, bool must have the bit pattern 0x00 or 0x01, and char must have a bit pattern in the range 0x0000_0000..=0x0000_D7FF or the range 0x0000_E000..=0x0010_FFFF)

The following example program exhibits undefined behavior due to this (run it in debug mode and in release mode and you'll most likely see different results):

fn main() {
    let b: char = secrets::traits::Bytes::uninitialized();
    match b {
        // Note that these two patterns together include all valid char values
        '\x00'..='\u{10fffe}' => dbg!("char1"),
        '\x01'..='\u{10ffff}' => dbg!("char2"), // This prints in release mode on my machine (secrets 1.2.0, rustc 1.61.0)
        _ => dbg!("huh?"), // This prints in debug mode on my machine
    };
}
zachs18 added a commit to zachs18/secrets that referenced this issue May 25, 2022
`bool` and `char` are not able to have any arbitrary bit-pattern, so they should not implement `Bytes`.

fixes stouset#100
@zachs18 zachs18 linked a pull request May 25, 2022 that will close this issue
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

Successfully merging a pull request may close this issue.

1 participant