Elastic-Finance-DAO / eefi_contracts

0 stars 0 forks source link

[EVT-02M] Incorrect Calculation of AMPL Dust #94

Open stalker474 opened 2 weeks ago

stalker474 commented 2 weeks ago

EVT-02M: Incorrect Calculation of AMPL Dust

Type Severity Location
Logical Fault ElasticVault.sol:L421-L424, L444

Description:

The ElasticVault::sell function will incorrectly calculate the AMPL dust funds to refund to the token_storage contract as it will transfer them prior to disbursing the for_treasury amount.

This results in the ElasticVault::sell function transmitting funds meant for users to the treasury incorrectly.

Impact:

All ElasticVault::sell operations will tap into AMPL tokens meant for other purposes due to an incorrect dust refund statement being executed prior to all AMPL tokens being utilized.

Example:

function sell(uint256 minimalExpectedEEFI, uint256 minimalExpectedOHM) external nonReentrant() _onlyTrader returns (uint256 eefi_purchased, uint256 ohm_purchased) {
    uint256 balance = ampl_token.balanceOf(address(token_storage));
    uint256 for_eefi = balance.mul(TRADE_POSITIVE_EEFI_100).divDown(100);
    uint256 for_ohm = balance.mul(TRADE_POSITIVE_OHM_100).divDown(100);
    uint256 for_treasury = balance.mul(TRADE_POSITIVE_TREASURY_100).divDown(100);

    uint256 ampl_balance = ampl_token.balanceOf(address(this));
    token_storage.claim(address(ampl_token));

    ampl_token.approve(address(trader), for_eefi.add(for_ohm));
    // buy EEFI
    eefi_purchased = trader.sellAMPLForEEFI(for_eefi, minimalExpectedEEFI);
    // buy OHM
    ohm_purchased = trader.sellAMPLForOHM(for_ohm, minimalExpectedOHM);
    // send AMPL dust back to the token_storage so shares accounting remains correct to the dot
    uint256 ampl_dust = ampl_token.balanceOf(address(this)).sub(ampl_balance);
    if(ampl_dust > 0) {
        ampl_token.safeTransfer(address(token_storage), ampl_dust);
    }

    // 10% of purchased EEFI is sent to the DAO Treasury.
    IERC20(address(eefi_token)).safeTransfer(treasury, eefi_purchased.mul(TREASURY_EEFI_100).divDown(100));
    // burn the rest
    uint256 to_burn = eefi_token.balanceOf(address(this));
    emit Burn(to_burn);
    IEEFIToken(address(eefi_token)).burn(to_burn);

    // distribute ohm to vaults
    uint256 to_rewards = ohm_purchased.mul(TRADE_POSITIVE_OHM_REWARDS_100).divDown(100);
    uint256 to_lp_staking = ohm_purchased.mul(TRADE_POSITIVE_LPSTAKING_100).divDown(100);
    ohm_token.approve(address(rewards_ohm), to_rewards);
    rewards_ohm.distribute(to_rewards, address(this));
    ohm_token.safeTransfer(address(staking_pool), to_lp_staking);
    staking_pool.forward();

    // distribute the remainder of OHM to the DAO treasury
    ohm_token.safeTransfer(treasury, ohm_token.balanceOf(address(this)));
    // distribute the remainder of AMPL to the DAO treasury
    ampl_token.safeTransfer(treasury, for_treasury);
}

Recommendation:

We advise the system to perform the for_treasury transfer prior to the ampl_dust code block, ensuring that any AMPL that was not sold is properly refunded to the token_storage contract.

stalker474 commented 2 weeks ago

good catch, thanks