sherlock-audit / 2024-05-elfi-protocol-judging

11 stars 7 forks source link

ZeroTrust - ElfiToken and ElfiUSD Token do not require users to transfer or authorize; they can be directly burned #283

Closed sherlock-admin2 closed 4 months ago

sherlock-admin2 commented 4 months ago

ZeroTrust

Medium

ElfiToken and ElfiUSD Token do not require users to transfer or authorize; they can be directly burned

Summary

ElfiToken and ElfiUSD Token do not require users to transfer or authorize; they can be directly burned

Vulnerability Detail

 function _executeRedeemStakeToken(
        LpPool.Props storage pool,
        Redeem.Request memory params,
        address baseToken
    ) internal returns (uint256) {
        ExecuteRedeemCache memory cache;
        cache.poolValue = pool.getPoolValue();
        cache.totalSupply = TokenUtils.totalSupply(pool.stakeToken);
        cache.tokenDecimals = TokenUtils.decimals(baseToken);
        if (cache.poolValue == 0 || cache.totalSupply == 0) {//@audit poolValue==0 完全有可能
            revert Errors.RedeemWithAmountNotEnough(params.account, baseToken);
        }

        cache.unStakeUsd = params.unStakeAmount.mul(cache.poolValue).div(cache.totalSupply);
        cache.redeemTokenAmount = CalUtils.usdToToken(
            cache.unStakeUsd,
            cache.tokenDecimals,
            OracleProcess.getLatestUsdUintPrice(baseToken, false)
        );

        if (pool.getPoolAvailableLiquidity() < cache.redeemTokenAmount) {
            revert Errors.RedeemWithAmountNotEnough(params.account, params.redeemToken);
        }

        if (params.minRedeemAmount > 0 && cache.redeemTokenAmount < params.minRedeemAmount) {
            revert Errors.RedeemStakeTokenTooSmall(cache.redeemTokenAmount);
        }

        StakingAccount.Props storage stakingAccountProps = StakingAccount.load(params.account);
        AppPoolConfig.LpPoolConfig memory poolConfig = AppPoolConfig.getLpPoolConfig(pool.stakeToken);
        uint256 redeemFee = FeeQueryProcess.calcMintOrRedeemFee(cache.redeemTokenAmount, poolConfig.redeemFeeRate);
        FeeProcess.chargeMintOrRedeemFee(
            redeemFee,
            params.stakeToken,
            params.redeemToken,
            params.account,
            FeeProcess.FEE_REDEEM,
            false
        );
        VaultProcess.transferOut(
            params.stakeToken,
            params.redeemToken,
            params.receiver,
            cache.redeemTokenAmount - cache.redeemFee
        );
        pool.subPoolAmount(pool.baseToken, cache.redeemTokenAmount);
@>>        StakeToken(params.stakeToken).burn(params.account, params.unStakeAmount);
        stakingAccountProps.subStakeAmount(params.stakeToken, params.unStakeAmount);

        return cache.redeemTokenAmount;
    }
function _executeRedeemStakeUsd(
        UsdPool.Props storage pool,
        address stakeUsdToken,
        Redeem.Request memory params
    ) internal returns (uint256) {
        address account = params.account;
        uint256 poolValue = pool.getUsdPoolValue();
        uint256 totalSupply = TokenUtils.totalSupply(stakeUsdToken);
        if (poolValue == 0 || totalSupply == 0) {
            revert Errors.RedeemWithAmountNotEnough(account, params.redeemToken);
        }
        uint8 tokenDecimals = TokenUtils.decimals(params.redeemToken);
        uint256 unStakeUsd = params.unStakeAmount.mul(poolValue).div(totalSupply);
        uint256 redeemTokenAmount = CalUtils.usdToToken(
            unStakeUsd,
            tokenDecimals,
            OracleProcess.getLatestUsdUintPrice(params.redeemToken, false)
        );
        if (params.minRedeemAmount > 0 && redeemTokenAmount < params.minRedeemAmount) {
            revert Errors.RedeemStakeTokenTooSmall(redeemTokenAmount);
        }
        if (pool.getMaxWithdraw(params.redeemToken) < redeemTokenAmount) {
            revert Errors.RedeemWithAmountNotEnough(params.account, params.redeemToken);
        }

        uint256 redeemFee = FeeQueryProcess.calcMintOrRedeemFee(
            redeemTokenAmount,
            AppPoolConfig.getUsdPoolConfig().redeemFeeRate
        );
        FeeProcess.chargeMintOrRedeemFee(
            redeemFee,
            params.stakeToken,
            params.redeemToken,
            params.account,
            FeeProcess.FEE_REDEEM,
            false
        );

@>>        StakeToken(params.stakeToken).burn(account, params.unStakeAmount);
        StakeToken(params.stakeToken).transferOut(params.redeemToken, params.receiver, redeemTokenAmount - redeemFee);
        return redeemTokenAmount;
    }

Since this protocol allows for contract upgrades, Deploy Role( used for smart contract deployment and dynamic upgrades) is RESTRICTED. There is a risk of transferring funds and burning ElfiToken and ElfiUSD Tokens.

Impact

There is a risk of transferring funds and burning ElfiToken and ElfiUSD Tokens.

Code Snippet

https://github.com/sherlock-audit/2024-05-elfi-protocol/blob/main/elfi-perp-contracts/contracts/process/RedeemProcess.sol#L133

https://github.com/sherlock-audit/2024-05-elfi-protocol/blob/main/elfi-perp-contracts/contracts/process/RedeemProcess.sol#L185

Tool used

Manual Review

Recommendation

Add user-related authorization before tokens can be burned.

0xELFi commented 4 months ago

Only the keeper can call the execute function, and the burn function of the stakeToken can only be called by the diamond smart contract. Therefore, there are permission restrictions in place.

nevillehuang commented 4 months ago

Invalid, not sure what this issue is trying to point to. Users burn tokens only when you the keeper executes the users redemption request