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

Consider improving soundness of trait impl macros #61

Open
jhand2 opened this issue Dec 8, 2023 · 0 comments
Open

Consider improving soundness of trait impl macros #61

jhand2 opened this issue Dec 8, 2023 · 0 comments

Comments

@jhand2
Copy link

jhand2 commented Dec 8, 2023

Several of the impl macros are technically unsound (although their usages do not lead to undefined behavior).

Take hex_format as an example:

ufmt/src/impls/hex.rs

Lines 3 to 20 in d95831b

macro_rules! hex_format {
($buf:expr, $val:expr, $options:expr) => {{
let mut cursor = $buf.len();
let mut val = $val;
if val <= 0 {
cursor -= 1;
$buf[cursor] = b'0';
} else {
while val != 0 && cursor > 0 {
let rem = val & 0xf;
cursor -= 1;
$buf[cursor] = hex_digit(rem as u8, $options.upper_case);
val >>= 4;
}
}
unsafe { core::str::from_utf8_unchecked(&$buf[cursor..]) }
}};
}

Since it does not place any requirements on the input type besides having a len method, an IndexMut impl, and a Deref<Target=[u8]> impl, the returned values from these methods may not actually be valid (e.g. the user could write a type which has a len() method that returns 500 and a Deref impl that returns a zero-sized buffer).

However this is ultimately mitigated because the usage of this macro is restricted to only fixed-size byte buffers which are sized for primitives. It may be worth considering doing more type validation on the macro inputs so that the safety of these macros doesn't depend on the specific usages.

The following macros have similar properties:

  • ixx!
  • uxx!

Additionally, these and other macros have similar behavior where if they are passed 0-sized buffers, lines like this would lead to undefined behavior:

let mut i = len - 1;

This safety issue is also mitigated because the macro is not exported, but generic use of this macro wouldn't be sound.

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