search

cenotelie / hime

Apache License 2.0
27 stars 4 forks source link

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

Open ethindp opened 1 year ago

ethindp commented 1 year ago

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.

woutersl commented 12 months ago

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.