An edge case in the TwabController._transferBalance can cause total balance for a vault account to decrease although it did not actually decrease. This will cause the sum of individual delegateBalances for a vault to be greater than the registered total for that vault. This again will skew the odds in favor of winning a price, causing the reserve to be drained faster over time than intended.
Proof of Concept
The TwabController._transferBalance function handles the case incorrectly where _to is equal to the SPONSORSHIP_ADDRESS. First, assume the _from address is an address that has a balance and delegates to itself (the default). Then the function will decrease the balance and delegateBalance of _from. It will also decrease the total balances of the vault account (note that _toDelegate will be SPONSORSHIP_ADDRESS due to default):
Then, the balance and delegateBalance of SPONSORSHIP_ADDRESS will be increased by the same amount. This is not supposed to happen in the first place, as this address is not meant to have any balances. However, the issue described under #impact is due to the fact that the total balances of the vault account will not be adjusted, because the condition _toDelegate != SPONSORSHIP_ADDRESS is not met:
The ratio of _userTwab and _vaultTwabTotalSupply plays a role when determining a winner in TierCalculationLib.isWinner and the underlying model assumes the invariant to hold true:
Lines of code
https://github.com/GenerationSoftware/pt-v5-twab-controller/blob/0145eeac23301ee5338c659422dd6d69234f5d50/src/TwabController.sol#L554-L570
Vulnerability details
Impact
An edge case in the
TwabController._transferBalance
can cause total balance for a vault account to decrease although it did not actually decrease. This will cause the sum of individual delegateBalances for a vault to be greater than the registered total for that vault. This again will skew the odds in favor of winning a price, causing the reserve to be drained faster over time than intended.Proof of Concept
The
TwabController._transferBalance
function handles the case incorrectly where_to
is equal to theSPONSORSHIP_ADDRESS
. First, assume the_from
address is an address that has a balance and delegates to itself (the default). Then the function will decrease the balance and delegateBalance of_from
. It will also decrease the total balances of the vault account (note that_toDelegate
will beSPONSORSHIP_ADDRESS
due to default):Then, the balance and delegateBalance of
SPONSORSHIP_ADDRESS
will be increased by the same amount. This is not supposed to happen in the first place, as this address is not meant to have any balances. However, the issue described under #impact is due to the fact that the total balances of the vault account will not be adjusted, because the condition_toDelegate != SPONSORSHIP_ADDRESS
is not met:The ratio of
_userTwab
and_vaultTwabTotalSupply
plays a role when determining a winner inTierCalculationLib.isWinner
and the underlying model assumes the invariant to hold true:If the sum of individual twabs is higher than the sum, there will be more winners than intended, causing the described drainage of the reserve.
Tools Used
Manual Review
Recommended Mitigation Steps
Disallow
_to
to beSPONSORSHIP_ADDRESS
Assessed type
Invalid Validation