sherlock-audit / 2023-01-derby-judging

4 stars 1 forks source link

Saeedalipoor01988 - Yearn Vault using _shares to processd withdraws from vault, not amount of asset #219

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

Saeedalipoor01988

high

Yearn Vault using _shares to processd withdraws from vault, not amount of asset

Summary

based on the yVault.sol contract, this contract is using the number of shares in withdraw function and not the number of assets.

Vulnerability Detail

if you check yVault.sol contract, at yVault.sol#L101, withdraw function is using (uint256 _shares) to process withdraw requests,

function withdraw(uint256 _shares) public {
}

but in the YearnProvider.sol#L45 from derby-yield-optimiser, for process withdraw, we get _amount as input and based on comment, _amount = Amount to withdraw.

/// @notice Withdraw the underlying asset from Yearn /// @dev Pulls cTokens from Vault, redeem them from Yearn, send underlying back. /// @param _amount Amount to withdraw /// @param _yToken Address of protocol LP Token eg yUSDC /// @param _uToken Address of underlying Token eg USDC /// @return Underlying tokens received and sent to vault e.g USDC function withdraw( uint256 _amount, address _yToken, address _uToken ) external override returns (uint256) {

Impact

check below finding please : https://github.com/code-423n4/2022-07-swivel-findings/issues/43

Code Snippet

https://github.com/sherlock-audit/2023-01-derby/blob/main/derby-yield-optimiser/contracts/Providers/YearnProvider.sol#L45 https://github.com/sherlock-audit/2023-01-derby/blob/main/derby-yield-optimiser/contracts/Providers/YearnProvider.sol#L40 https://github.com/sherlock-audit/2023-01-derby/blob/main/derby-yield-optimiser/contracts/Providers/YearnProvider.sol#L56 https://github.com/yearn/yearn-protocol/blob/7b7d4042f87d6e854b00de9228c01f6f587cf0c0/contracts/vaults/yVault.sol#L101

Tool used

Manual Review

Recommendation

In the withdraw() function in YearnProvider.sol, calculate the price per share and use that to retrieve the correct number of underlying assets.

mister0y commented 1 year ago

We use shares in the withdrawFromProtocol in the vault.