Closed code423n4 closed 11 months ago
Picodes marked the issue as primary issue
asselstine marked the issue as sponsor confirmed
Keeping High severity, considering this is an instance of "loss of yield"
Picodes marked the issue as satisfactory
Picodes marked issue #408 as primary and marked this issue as a duplicate of 408
I've removed the receiver
param in the following PR: https://github.com/GenerationSoftware/pt-v5-vault/pull/19
This way, only the msg.sender
can sponsor the Vault by depositing into it and delegating to the sponsorship address if it is not already the case.
If the user wants to deposit on behalf of another user, he can still use the deposit
function. Funds will then be delegated to any address set by the receiver
.
Lines of code
https://github.com/GenerationSoftware/pt-v5-vault/blob/b1deb5d494c25f885c34c83f014c8a855c5e2749/src/Vault.sol#L480-L482 https://github.com/GenerationSoftware/pt-v5-vault/blob/b1deb5d494c25f885c34c83f014c8a855c5e2749/src/Vault.sol#L494-L504 https://github.com/GenerationSoftware/pt-v5-vault/blob/b1deb5d494c25f885c34c83f014c8a855c5e2749/src/Vault.sol#L982-L994
Vulnerability details
Impact
An attacker can change the delegatee of a user who deposited into the vault to the
SPONSORSHIP_ADDRESS
address by calling one of the functionssponsor
orsponsorWithPermit
and giving the address of the user as_receiver
.The impact of this issue is that the attacker can stop users from winning prizes because the users who have delegated to the
SPONSORSHIP_ADDRESS
address get their chances of winning prizes revoked.Proof of Concept
As the functions
sponsor
andsponsorWithPermit
do the same thing except the later include a permit functionality which doesn't have an impact on this issue, i will explain how this issue can be exploited with thesponsor
function and the same apply to thesponsorWithPermit
function.We start with the
sponsor
function :As you can see the function can be called by anyone, and it takes the
_assets
amount of asset to deposit into the vault and a_receiver
address that the user wants to deposit to, the function calls the internal function_sponsor
below :Inside this function we can see that it call the
deposit
function for performing the deposit of the asset amount, and then the function checks the current delegatee of the_receiver
address, if this one is different from theSPONSORSHIP_ADDRESS
address the function will call the_twabController.sponsor
function to changes the_receiver
delegatee address to theSPONSORSHIP_ADDRESS
address :In the comments of the
TwabController
contract we can see the following :Basically any users who is delegating to the
SPONSORSHIP_ADDRESS
address is not included in the prize distribution and has no chances to win.So to resume any attacker can change all other users delegations to the
SPONSORSHIP_ADDRESS
address and deprive them from the chance of winning prizes, and this attack will not cost the attacker any funds as he can at each time deposit a very small amount to every user when calling thesponsor
function.The scenario of the attack can be summarized as follows :
Alice has deposited funds into a given Vault, and she has not delegated them so in the
TwabController
contract she is basically delegating to her self meaning :delegates[_vault][Alice] = Alice
.The attacker Bob sees that Alice has deposited funds into the vault, so he calls the
sponsor
function with the following parameters :_assets = 10
(choose 10 just for the example but could be any other very small amount) and_receiver = Alice
.The
sponsor
function after callingdeposit
to deposit_assets = 10
to Alice, proceeds to check the current delegatee of Alice and it finds that it is different from theSPONSORSHIP_ADDRESS
address :delegates[_vault][Alice] = Alice != SPONSORSHIP_ADDRESS
So it calls the
_twabController.sponsor
function to change Alice's delegated address toSPONSORSHIP_ADDRESS
.The attack now is over and Bob managed to change Alice delegatee to
SPONSORSHIP_ADDRESS
meaning that now she is not included in the prize distribution anymore (has 0 chances to win).Alice could find out about this after a while and change the delegation but until that time she will be thinking that she's participating in the lottery when in fact she's not. And Bob can repeat the same attack on other users who also deposited into the Vault.
Tools Used
Manual review
Recommended Mitigation Steps
There are many ways to solve this issue but the simplest one is to add a check in the
_sponsor
function which changes the delegation only if :_receiver == msg.sender
.The function
_sponsor
should be updated as follows :Assessed type
Other