search

code-423n4 / 2022-02-anchor-findings

0 stars 0 forks source link

Users will not be able to unbond due to error in calculation for historical time #75

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-02-anchor/blob/7af353e3234837979a19ddc8093dc9ad3c63ab6b/contracts/anchor-bAsset-contracts/contracts/anchor_basset_hub/src/contract.rs#L523 (https://github.com/code-423n4/2022-02-anchor/blob/7af353e3234837979a19ddc8093dc9ad3c63ab6b/contracts/anchor-bAsset-contracts/contracts/anchor_basset_hub/src/state.rs#L167) (https://github.com/code-423n4/2022-02-anchor/blame/7af353e3234837979a19ddc8093dc9ad3c63ab6b/contracts/anchor-bAsset-contracts/contracts/anchor_basset_hub/src/contract.rs#L527)

Vulnerability details

Since params.unbonding_period is some time in the future, the value would be larger than the current timestamp in seconds and would lead to an underflow. The unbonding_period should be the first operand and not the second. Also, in instantiate() , ensure the period is >0 and that when the period ends, timestamp should be ≤ unbonding_period.

Since this calculation uses the incorrect value above :

(https://github.com/code-423n4/2022-02-anchor/blob/7af353e3234837979a19ddc8093dc9ad3c63ab6b/contracts/anchor-bAsset-contracts/contracts/anchor_basset_hub/src/state.rs#L167)

This will return 0 since the time the user wants to unbond will be > block_time(historical_time) :

(https://github.com/code-423n4/2022-02-anchor/blame/7af353e3234837979a19ddc8093dc9ad3c63ab6b/contracts/anchor-bAsset-contracts/contracts/anchor_basset_hub/src/contract.rs#L527)

This will lead to user not being able to withdraw their bonds.

Use check_to_be() to compare both uints to ensure that they don't underflow.

bitn8 commented 2 years ago

This is not a issue.

GalloDaSballo commented 2 years ago

After looking into it, I believe that a revert is ungraceful but correct as you can't withdraw before the unbonding_period has passed.

As such a revert there is not only extremely unlikely (unbending period would need to be crazy long), but preferable as the tokens are locked.

For those reasons, I think the sponsor is right in disputing

albertchon commented 2 years ago

Agreed, not an issue.