search

diwakergupta / stacks-blockchain-tob-audit

GNU General Public License v3.0
0 stars 0 forks source link

Incorrect implementations of `From<T>` for `MARFValue` #1

Open bradlarsen opened 4 years ago

bradlarsen commented 4 years ago

There are several implementations of the From<T> trait for MARFValue that can fail. Implementations of this trait must not fail. In these cases, the TryFrom<T> trait should be used instead.

https://doc.rust-lang.org/std/convert/trait.From.html https://doc.rust-lang.org/std/convert/trait.TryFrom.html

Locations of From<T> implementations that can fail:

https://github.com/trailofbits/x-audit-blockstack-core/blob/e2d3d5bae539d242851620e28129af6c4a9de642/src/chainstate/stacks/index/mod.rs#L121-L133

https://github.com/trailofbits/x-audit-blockstack-core/blob/e2d3d5bae539d242851620e28129af6c4a9de642/src/chainstate/stacks/index/mod.rs#L35-L147

https://github.com/trailofbits/x-audit-blockstack-core/blob/e2d3d5bae539d242851620e28129af6c4a9de642/src/chainstate/stacks/index/mod.rs#L149-L163

https://github.com/trailofbits/x-audit-blockstack-core/blob/e2d3d5bae539d242851620e28129af6c4a9de642/src/chainstate/stacks/index/mod.rs#L165-L179

jcnelson commented 4 years ago

Thank you for raising this. I believe panic!ing is the intended behavior here. In each case, the conditions that lead to the panic! are indicative of irrecoverable chain state corruption, and the best thing to do would be to abort and (TBD) prompt the user to run a recovery tool.