By default users delegate their balance to address(0) which corresponds to the user delegating their balance to themselves. Consequently, it is likely that a user would reset their delegation to themselves (after delegating to another user) by delegating back to the 0 address. However the previous implementation contained a bug where the user balance would actually be moved to the 0 address, thereby losing their vault balance forever.
Mitigation
With the updated implementation, delegating to the 0 address now has the same behaviour as un-delegating (i.e. delegating to the sponsorship address). If a user wanted to delegate their balance back to their own address they can simply pass in their own address. This has the same effect as the initial 0 delegation that also defaults to the user address. The original issue of funds being lost is now protected against.
Lines of code
Vulnerability details
Comments
By default users delegate their balance to
address(0)
which corresponds to the user delegating their balance to themselves. Consequently, it is likely that a user would reset their delegation to themselves (after delegating to another user) by delegating back to the 0 address. However the previous implementation contained a bug where the user balance would actually be moved to the 0 address, thereby losing their vault balance forever.Mitigation
With the updated implementation, delegating to the 0 address now has the same behaviour as un-delegating (i.e. delegating to the sponsorship address). If a user wanted to delegate their balance back to their own address they can simply pass in their own address. This has the same effect as the initial 0 delegation that also defaults to the user address. The original issue of funds being lost is now protected against.
If I'm being pedantic there should be a small change to https://github.com/GenerationSoftware/pt-v5-twab-controller/blob/main/src/TwabController.sol#L658 from
_to
toto
, but there isn't actually a material impact of this change.Conclusion
LGTM