Lines of code

Vulnerability details

Impact

The MAX_ORDERS constant is defined as a uint96, which has a maximum value of 2^96-1. This means that the maximum number of orders that the contract is able to handle is 2^96-1. However, if the number of orders exceeds this maximum value, the calculation for the minimum buy amount per order will be incorrect and could lead to confusion and errors in the trading process.

Proof of Concept

It could be exploited by a malicious actor submitting an order with a high number of orders, exceeding the maximum value of 2^96-1. This could cause confusion in the trading process and potentially lead to errors such as the minimum buy amount per order being calculated incorrectly.

contract GnosisTrade {
    uint96 public constant MAX_ORDERS = 1e5;

    function submitOrder(uint96 numberOfOrders) public {
        require(numberOfOrders <= MAX_ORDERS, "Number of orders exceeds maximum value of MAX_ORDERS");
        // code to submit order
    }
}

// Malicious actor submits an order with a high number of orders
contract Attacker {
    GnosisTrade gnosisTrade;
    constructor(address _gnosisTrade) public {
        gnosisTrade = GnosisTrade(_gnosisTrade);
    }

    function exploit() public {
        gnosisTrade.submitOrder(2**96);
    }
}

The above exploit function would cause a require statement to fail in the GnosisTrade contract, as the number of orders exceeds the maximum value of MAX_ORDERS.

Tools Used

Manual audit.

Recommended Mitigation Steps

Update the MAX_ORDERS constant to a lower value that is more appropriate for the expected number of orders in the trading process.
Additionally, it would be a good practice to add a comment explaining the reasoning behind the value chosen for MAX_ORDERS to make it clear for other developers.

Another solution would be to use SafeMath library to prevent integers overflow in calculations.

import "@openzeppelin/contracts/math/SafeMath.sol";

contract GnosisTrade {
    using SafeMath for uint96;

    uint96 public constant MAX_ORDERS = 1e5;
    ...
    function init(...) {
        ...
        require(req.sellAmount.mul(worstCasePrice).div(FEE_DENOMINATOR).div(MAX_ORDERS) <= sell.balanceOf(address(this)), "Insufficient balance of sell tokens");
        ...
    }
}

The text was updated successfully, but these errors were encountered:

All reactions