Core Contracts
Regnum Aurum Acquisition Corp
HardhatReal World AssetsNFT
77,280 USDC
View results
Previous Next
Submission Details
Severity: high
Valid

Invalid owner param when withdrawing from curve vault can bring DoS to LendingPool

glightspeed2

Summary

LendingPool maintains buffer ratio of provided liquidity. It deposits excess liquidity into curve vault when current liquidity ratio is above buffer ratio, and withdraws from curve vault when current liqudity ratio falls below buffer ratio. However, when withdrawing from curve vault, the owner parameter is incorrectly set to msg.sender. Due to this, liquidity rebalancing and ensuring will revert, which will lead to deposit/withdraw failure.

Vulnerability Details

The vulnerability resides in LendingPool._withdrawFromVault

function _withdrawFromVault(uint256 amount) internal {
curveVault.withdraw(amount, address(this), msg.sender, 0, new address[](0)); // @audit msg.sender is set to owner
totalVaultDeposits -= amount;
}

As we can see in ICurveCrvUSDVault.withdraw, owner parameter is set to msg.sender:

function withdraw(
uint256 assets,
address receiver,
address owner,
uint256 maxLoss,
address[] calldata strategies
) external returns (uint256 shares);

It depends on ICurveCrvUSDVault implementation but it will likely to pass those params to curve vault, which is ERC-4626 compliant.

Thus, it will try to burn shares of msg.sender i.e. user, instead of burning shares of LendingPool or RToken.

We can confirm the above statement by looking into curve vault source code:

@external
@nonreentrant('lock')
def withdraw(assets: uint256, receiver: address = msg.sender, owner: address = msg.sender) -> uint256:
"""
@notice Withdraw given amount of asset and burn the corresponding amount of vault shares
@param assets Amount of assets to withdraw
@param receiver Receiver of the assets (optional, sender if not specified)
@param owner Owner who's shares the caller takes. Only can take those if owner gave the approval to the sender. Optional
"""
total_assets: uint256 = self._total_assets()
assert total_assets - assets >= MIN_ASSETS or total_assets == assets, "Need more assets"
shares: uint256 = self._convert_to_shares(assets, False, total_assets)
if owner != msg.sender:
allowance: uint256 = self.allowance[owner][msg.sender]
if allowance != max_value(uint256):
self._approve(owner, msg.sender, allowance - shares)
​
controller: Controller = self.controller
@> self._burn(owner, shares) ## @audit burn from owner
assert self.borrowed_token.transferFrom(controller.address, receiver, assets, default_return_value=True)
controller.save_rate()
log Withdraw(msg.sender, receiver, owner, assets, shares)
return shares

Impact

  • Deposit and withdraw will revert when current liquidity ratio is below buffer ratio

  • In worst case, if user approved LendingPool to spend their curve share, their share will be burnt unexpectedly

Tools Used

Manual Review

Recommendations

owner parameter should be set to correct one. LendingPool, RToken or ICurveCrvUSDVault depending on curve vault implementation.

Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 month ago
Submission Judgement Published
Validated
Assigned finding tags:

LendingPool::_withdrawFromVault incorrectly uses msg.sender instead of address(this) as the owner parameter, causing vault withdrawals to fail

inallhonesty Lead Judge about 1 month ago
Submission Judgement Published
Validated
Assigned finding tags:

LendingPool::_withdrawFromVault incorrectly uses msg.sender instead of address(this) as the owner parameter, causing vault withdrawals to fail

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.