`VestingWallet` bricked if aggregate of transfers in exceed `type(uint256).max`
If VestingWallet receives over type(uint256).max, then the function below will revert every time it is called, effectively bricking the vesting wallet.
https://github.com/OpenZeppelin/openzeppelin-contracts/blob/5def3f7c7ef13e06bf868474f258a6e232c9ecec/contracts/finance/VestingWallet.sol#L139-L144
A user can send this value by doing the following:
- Transfer the total supply of a token to the vesting wallet of amount
type(uint256).max - Wait for some to vest and claim it
- Transfer the claimed amount back to the vesting wallet
This is something we discussed during development, and we figured out would not happen in any "realistic" use case:
- Let assume a token with 36 decimals (that is a lot, but possible in the context of DeFi/vaults)
- Lets assume there are 1 billion billions (10¹⁸) tokens in supply (that really is a lot)
You would need to transfer all the total supply 10²³ times before a uint256 overflow.
Said otherwize: If you transfered the total supply of that token (again, 10¹⁸ tokens with 36 decimals) in and out of the vesting contract 1000 times a second, it would still take you 3671 billions years (so about 266 times the age of the universe) to overflow the uint256.
2²⁵⁶ is realllly a big number !
This is something we discussed during development, and we figured out would not happen in any "realistic" use case:
- Let assume a token with 36 decimals (that is a lot, but possible in the context of DeFi/vaults)
- Lets assume there are 1 billion billions (10¹⁸) tokens in supply (that really is a lot)
You would need to transfer all the total supply 10²³ times before a uint256 overflow.
Said otherwize: If you transfered the total supply of that token (again, 10¹⁸ tokens with 36 decimals) in and out of the vesting contract 1000 times a second, it would still take you 3671 billions years (so about 266 times the age of the universe) to overflow the uint256.
2²⁵⁶ is realllly a big number !
The total supply could easily be uint256.max and then bricking the wallet would be easy. I rediscovered this while building the confidential version (only uint64 there so more relevent). It was mediated by using larger values for intermediary calculations. Maybe we should just document this in the contract.
The total supply could easily be uint256.max
Our (past) analysis showed that real tokens are veryyyyyyy far from that. Do you know any real usecase were token have such a large supply ?
The total supply could easily be uint256.max
Our (past) analysis showed that real tokens are veryyyyyyy far from that. Do you know any real usecase were token have such a large supply ?
I do not. I'm just pointing out that it could happen.