code-423n4 / 2023-08-goodentry-findings

3 stars 2 forks source link

Its possible to force users to buy options #74

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-08-goodentry/blob/71c0c0eca8af957202ccdbf5ce2f2a514ffe2e24/contracts/PositionManager/OptionsPositionManager.sol#L39

Vulnerability details

Impact

Detailed description of the impact of this finding. Its possible to force users to buy options

Proof of Concept

Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept. On the flashloan call executeOperation there is no protection that currect contract called flashloan thus its possible to force any users to buy options

Whenever anyone calls buyOptions his msg.sender is encoding and later decoding in a callback executeOperation

    function buyOptions(
        uint poolId,
        address[] memory options,
        uint[] memory amounts,
        address[] memory sourceSwap
    )
    external
    {
        require(options.length == amounts.length && sourceSwap.length == options.length, "OPM: Array Length Mismatch");
        bytes memory params = abi.encode(0, poolId, msg.sender, sourceSwap);
        (ILendingPool LP,,,,) = getPoolAddresses(poolId);

        uint[] memory flashtype = new uint[](options.length);
        for (uint8 i = 0; i < options.length;) {
            flashtype[i] = 2;
            unchecked {i += 1;}
        }
        LP.flashLoan(address(this), options, amounts, flashtype, msg.sender, params, 0);
    }

OptionsPositionManager.sol#L161

But anybody can create a same function, but with this difference since there is no validation in a flashloan callback that address(this) is a caller

    function buyOptions(
        uint poolId,
        address[] memory options,
        uint[] memory amounts,
        address[] memory sourceSwap,
+        address user
    )
    external
    {
        require(options.length == amounts.length && sourceSwap.length == options.length, "OPM: Array Length Mismatch");
-        bytes memory params = abi.encode(0, poolId, msg.sender, sourceSwap);
+        bytes memory params = abi.encode(0, poolId, user, sourceSwap);
        (ILendingPool LP,,,,) = getPoolAddresses(poolId);

        uint[] memory flashtype = new uint[](options.length);
        for (uint8 i = 0; i < options.length;) {
            flashtype[i] = 2;
            unchecked {i += 1;}
        }
-        LP.flashLoan(address(this), options, amounts, flashtype, msg.sender, params, 0);
+        LP.flashLoan(address(OptionsPositionManager), options, amounts, flashtype, msg.sender, params, 0);
    }

Tools Used

Recommended Mitigation Steps

    function executeOperation(
        address[] calldata assets,
        uint256[] calldata amounts,
        uint256[] calldata premiums,
        address initiator,
        bytes calldata params
    ) override external returns (bool result) {
+       require(initiator == address(this));
        uint8 mode = abi.decode(params, (uint8));
        // Buy options
        if (mode == 0) {
            (, uint poolId, address user, address[] memory sourceSwap) = abi.decode(params, (uint8, uint, address, address[]));
            executeBuyOptions(poolId, assets, amounts, user, sourceSwap);
        } else {
        // Liquidate
            (, uint poolId, address user, address collateral) = abi.decode(params, (uint8, uint, address, address));
            executeLiquidation(poolId, assets, amounts, user, collateral);
        }
        result = true;
    }

Assessed type

Invalid Validation

c4-pre-sort commented 1 year ago

141345 marked the issue as duplicate of #110

c4-judge commented 1 year ago

gzeon-c4 marked the issue as unsatisfactory: Invalid