fix: unsafe usage of unwrap in supply #1103
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As reported by SRLabs:
[High] Unsafe usage of unwrap() in supply::begin_block
Summary
The conversion of
total_supply
toFixedPointNumber
exceeds accuracy and returns None, which crashes the blockchain onceunwrap()
is called.Issue details
The begin_block function from pallet supply will increase the Currency total_issuance based on the inflation rate, using deposit_creating.
Because total_supply can become too large, checked_from_integer will return None and the a panic is being raised because of the unwrap().
We tested the possibility of the crash by using the following values for the genesis of pallet supply:
By calling any extrinsic after 100 blocks (the value of
start_height
), theunwrap()
call might fail, potentially causing nodes to panic.Risk
Once the blockchain reaches the start_height block, if the inflation rate is too high, the chain will crash.
Mitigation
Review the logic of begin_block and replace the checked_from_integer function with an arithmetic safe function.