code-423n4 / 2021-06-tracer-findings

1 stars 0 forks source link

Missing validation matchOrders == true in executeTrade #126

Closed code423n4 closed 3 years ago

code423n4 commented 3 years ago

Handle

a_delamo

Vulnerability details

Impact

The method executeTrade on Trader.sol is calling matchOrders method of the TracerPerpetualSwaps contract. executeTrade is validating that the call was successful, but is missing to validate the return result == true. As a result, we will be adding invalid trade.


            (bool success, ) = makeOrder.market.call(
                abi.encodePacked(
                    ITracerPerpetualSwaps(makeOrder.market).matchOrders.selector,
                    abi.encode(makeOrder, takeOrder, fillAmount)
                )
            );

            // ignore orders that cannot be executed
            //FIXME: We should also check if return == false
            if (!success) continue;

            // update order state
            filled[makerOrderId] = makeOrderFilled + fillAmount;
            filled[takerOrderId] = takeOrderFilled + fillAmount;
            averageExecutionPrice[makerOrderId] = newMakeAverage;
            averageExecutionPrice[takerOrderId] = newTakeAverage;
                ....
                // validate orders can match, and outcome state is valid
        if (
            !Perpetuals.canMatch(order1, filled1, order2, filled2) ||
            !Balances.marginIsValid(
                newPos1,
                balances[order1.maker].lastUpdatedGasPrice *
                    LIQUIDATION_GAS_COST,
                pricingContract.fairPrice(),
                trueMaxLeverage()
            ) ||
            !Balances.marginIsValid(
                newPos2,
                balances[order2.maker].lastUpdatedGasPrice *
                    LIQUIDATION_GAS_COST,
                pricingContract.fairPrice(),
                trueMaxLeverage()
            )
        ) {
            // emit failed to match event and return false
            if (order1.side == Perpetuals.Side.Long) {
                emit FailedOrders(
                    order1.maker,
                    order2.maker,
                    order1Id,
                    order2Id
                );
            } else {
                emit FailedOrders(
                    order2.maker,
                    order1.maker,
                    order2Id,
                    order1Id
                );
            }
            return false;
        }
                ....
loudoguno commented 3 years ago

adding duplicate label (of #71 ) and closing as per judges sheet