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

Serious bug (4.2.4): binary manipulation code (bin.rs) has code that causes panics #82

Open
ethindp opened this issue Aug 21, 2023 · 1 comment
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@ethindp
Copy link

ethindp commented Aug 21, 2023

The code in bin.rs is unsafe and causes a panic (in read_u16/read_u32). It'd probably be a much better (and safer) idea to either use Nom or to interpret the bytes as a string and read the u16s/u32s via uX::from_str_radix.

@ethindp ethindp changed the title Serious bug (4.2.4): binary manipulation code is full of dangerous hacks Serious bug (4.2.4): binary manipulation code (bin.rs) has code that causes panics Aug 21, 2023
@woutersl woutersl self-assigned this Sep 18, 2023
@woutersl woutersl added the enhancement New feature or request label Sep 18, 2023
@woutersl
Copy link
Member

Hi, thanks for the feedback.
The first thing is that the code in bin.rs is there to rebuild u16 and u32 values from slices of bytes that are their binary representation. Those bytes are not strings, so that rules out uX::from_str_radix. uX::from_be_bytes would be more appropriate and would fix this problem.
To clarify a little bit, the code does not use unsafe, although it CAN panic in some cases when the data being loaded is not correct ; but it would argue this is an acceptable hypothesis, if the data from the automaton is garbage, this is catastrophic at runtime.

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

No branches or pull requests

2 participants