Skip to content

There is a calculation error in AuraVault::redeem(). #170

Open
@howlbot-integration

Description

@howlbot-integration

Lines of code

https://github.com/code-423n4/2024-07-loopfi/blob/57871f64bdea450c1f04c9a53dc1a78223719164/src/vendor/AuraVault.sol#L256

Vulnerability details

Impact

The amount of funds that users can withdraw decreases, leading to a loss of funds for users.

Proof of Concept

  function redeem(
        uint256 shares,
        address receiver,
        address owner
    ) public virtual override(IERC4626, ERC4626) returns (uint256) {
        require(shares <= maxRedeem(owner), "ERC4626: redeem more than max");

        // Redeem assets from Aura reward pool and send to "receiver"
@>>        uint256 assets = IPool(rewardPool).redeem(shares, address(this), address(this));

        _withdraw(_msgSender(), receiver, owner, assets, shares);

        return assets;
    }

We can see that AuraVault::redeem() confuses AuraVault’s shares with rewardPool’s shares. AuraVault’s shares need to be converted into AuraVault’s underlying tokens (assets) before they can be withdrawn. This is particularly problematic because, as we know from the rewardPool contract address, rewardPool::redeem() functions the same way as rewardPool::withdraw().

https://vscode.blockscan.com/ethereum/0x00A7BA8Ae7bca0B10A32Ea1f8e2a1Da980c6CAd2

 function redeem(
        uint256 shares,
        address receiver,
        address owner
    ) external virtual override returns (uint256) {
        return withdraw(shares, receiver, owner);
    }

Moreover, in the rewardPool, the ratio of share to asset is always 1:1.

Scenario Example:

Let’s assume that in AuraVault, the ratio of share to asset is always 1:2.

In this case, if a user withdraws 1 share, they will ultimately receive only 1 asset, whereas they should have received 2 assets.

Tools Used

Manual Review

Recommended Mitigation Steps

  function redeem(
        uint256 shares,
        address receiver,
        address owner
    ) public virtual override(IERC4626, ERC4626) returns (uint256) {
        require(shares <= maxRedeem(owner), "ERC4626: redeem more than max");
+        uint256 assets = previewRedeem(shares);

        // Redeem assets from Aura reward pool and send to "receiver"
-       uint256 assets = IPool(rewardPool).redeem(shares, address(this), address(this));
+        assets = IPool(rewardPool).redeem(assets, address(this), address(this));

        _withdraw(_msgSender(), receiver, owner, assets, shares);

        return assets;
    }

Assessed type

Error

Metadata

Metadata

Assignees

No one assigned

    Labels

    3 (High Risk)Assets can be stolen/lost/compromised directly🤖_03_groupAI based duplicate group recommendationH-05bugSomething isn't workingprimary issueHighest quality submission among a set of duplicatessatisfactorysatisfies C4 submission criteria; eligible for awardsselected for reportThis submission will be included/highlighted in the audit reportsponsor acknowledgedTechnically the issue is correct, but we're not going to resolve it for XYZ reasonssufficient quality reportThis report is of sufficient quality

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions