Open code423n4 opened 12 months ago
Picodes marked the issue as primary issue
asselstine marked the issue as sponsor confirmed
Picodes marked the issue as satisfactory
Picodes marked the issue as selected for report
Out of scope per the automated report: https://gist.github.com/itsmetechjay/e7fd03943bbacff1984a33b9f89c4149 (MEDIUM-4)
Picodes marked the issue as unsatisfactory: Out of scope
@Picodes, The fee on transfer token vulnerability described in this issue (#470) is related to the token transfers happening in the Vault.sol
contract. How the fee charged on token transfers could effect the Vault._deposit
, _yieldVault.deposit
and _yieldVault.withdraw
functions.
The MEDIUM-4
given in the automated report
has found one instance of this vulnerability (fee on token transfer) and it is related to the PrizePool.sol
contract and not the Vault.sol
contract. It is not the same vulnerability present in the #470 issue.
Indeed. The automated report didn't properly flag that Vault.sol
won't work with FoT.
Picodes marked the issue as selected for report
Picodes marked the issue as satisfactory
Lines of code
https://github.com/GenerationSoftware/pt-v5-vault/blob/b1deb5d494c25f885c34c83f014c8a855c5e2749/src/Vault.sol#L951-L956 https://github.com/GenerationSoftware/pt-v5-vault/blob/b1deb5d494c25f885c34c83f014c8a855c5e2749/src/Vault.sol#L959 https://github.com/GenerationSoftware/pt-v5-vault/blob/b1deb5d494c25f885c34c83f014c8a855c5e2749/src/Vault.sol#L1026-L1027
Vulnerability details
Impact
The
Vault._deposit
function is used by the users to deposit_assets
to the vault and mint vault shares to therecipient
address. The amount of_assets
are transferred to theVault
as follows:The
Vault.deposit
function uses this_assets
amount to calculate the number ofshares
to be minted to the_recipient
address.The issue here is if the underlying
_asset
is a fee on transfer token then the actual received amount to the vault will be less than what is referred in theVault.deposit
function_assets
input parameter. But the shares to mint is calculated using the entire_assets
amount.This issue could be further aggravated since the
_asset
is againdeposited
to the_yieldVault
and when needing to be redeemed will bewithdrawn
from the_yieldVault
as well. These operations will again charge a fee if the_asset
is a fee on transfer token. Hence the actual left_asset
amount for particular user will be less than the amount he initially transferred in.Hence when the user
redeems
the minted shares back to the_assets
, the contract will not have enough assets to transfer to theredeemer
thus reverting the transaction.Proof of Concept
https://github.com/GenerationSoftware/pt-v5-vault/blob/b1deb5d494c25f885c34c83f014c8a855c5e2749/src/Vault.sol#L951-L956
https://github.com/GenerationSoftware/pt-v5-vault/blob/b1deb5d494c25f885c34c83f014c8a855c5e2749/src/Vault.sol#L959
https://github.com/GenerationSoftware/pt-v5-vault/blob/b1deb5d494c25f885c34c83f014c8a855c5e2749/src/Vault.sol#L1026-L1027
Tools Used
Manual Review and VSCode
Recommended Mitigation Steps
Hence it is recommended to compute the
_assets
amount balance of the contract before and after thesafeTransferFrom
call and get the difference between the two as the actually transferred amount to theVault
. Then this actually transferred amount can be converted to shares and mint the correct amount of shares to therecipient
.Assessed type
Other