一

Summary

在执行 _getExchangeRate函数的时候，没有检查，tokenIn和tokenOut是不是同种的代币，导致一些无意义的汇率计算出来

Root Cause
2024-11-oku-HeYuan-33/oku-custom-order-types/contracts/automatedTrigger/AutomationMaster.sol

Lines 77 to 86 in ee3f781

function _getExchangeRate( 
    IERC20 tokenIn, 
    IERC20 tokenOut 
) internal view returns (uint256 exchangeRate) { 
    // Retrieve USD prices from oracles, scaled to 1e8 
    uint256 priceIn = oracles[tokenIn].currentValue(); 
    uint256 priceOut = oracles[tokenOut].currentValue(); 
 
    // Return the exchange rate in 1e8 terms 
    return (priceIn * 1e8) / priceOut;

This code calculates the exchange rate between two tokens, but it doesn’t check if the two tokens are the same. This oversight can affect the getMinAmountReceived function. When the same token is provided for both the input and output, no actual exchange happens, but due to the slippage mechanism, the user will see a reduced amount.
Example:

The user submits 100 DAI and targets an exchange of 100 DAI (i.e., both tokenIn and tokenOut are DAI).
The user sets a 1% slippage (i.e., slippageBips = 100), allowing for up to 1% price deviation.
minAmountReceived = (100 * (10000 - 100)) / 10000 = 99.0 DAI
This means that the user expects to receive at least 99 DAI, accounting for the 1% slippage.
However, since no actual token swap occurs (because both the input and output tokens are the same), the user still holds 100 DAI without any exchange taking place. The calculation for minAmountReceived is incorrect, as it assumes that a token swap has occurred, when in reality, there has been no exchange.
As a result, the system might show a reduced amount due to the slippage tolerance, even though no actual trade was made.

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

No response

Impact

Funds Loss:
If the contract does not prevent swapping the same token, users might make decisions based on incorrect minimum received amounts in other parts of the logic (e.g., through other contracts), leading to loss of funds.

Dependency on Vulnerable Contracts:
If other contracts rely on functions like getMinAmountReceived or similar exchange rate calculation functions to determine if a transaction should be executed, an attacker could exploit this vulnerability. By submitting transactions with the same token being swapped, this could lead to incorrect decision-making and improper execution of trades.

Gas Waste:
Attackers could initiate many such meaningless exchange transactions, which would consume a significant amount of gas without any real effect on the system. This results in increased gas costs for the network and potential denial of service for legitimate users.

Division by zero：

uint8 decimalIn = ERC20(address(tokenIn)).decimals();
      uint8 decimalOut = ERC20(address(tokenOut)).decimals();

      if (decimalIn > decimalOut) {
          // Reduce amountIn to match the lower decimals of tokenOut
          return amountIn / (10 ** (decimalIn - decimalOut));

PoC

No response

Mitigation

Add a check to verify whether tokenIn and tokenOut are the same token.

function _getExchangeRate(
      IERC20 tokenIn,
      IERC20 tokenOut
  ) internal view returns (uint256 exchangeRate) {
      // Retrieve USD prices from oracles, scaled to 1e8
      require(tokenIn != tokenOut, "Input and output tokens must be different");

      uint256 priceIn = oracles[tokenIn].currentValue();
      uint256 priceOut = oracles[tokenOut].currentValue();

      // Return the exchange rate in 1e8 terms
     
      return (priceIn * 1e8) / priceOut;
  }

这次是趁着期末，简单看了一下这个合约，没有太仔细的发现，就还是提交了一个中等的发现（除以零的情况），但是没有被认可，下面就来看看正确的发现吧

重入攻击（高）(第一次遇到在balanceOf填充恶意代码)

重入攻击,就是在fillOrder函数和 execute函数中，没有防止重入的修饰，而且也没有遵循 “检查-效果-交互”的模式，

function fillOrder(  
    uint96 pendingOrderIdx,  
    uint96 orderId,  
    address target,  
    bytes calldata txData  
) external override {  
    // 获取订单
    Order memory order = orders[orderId];  

    require(  
        order.orderId == pendingOrderIds[pendingOrderIdx],  
        "Order Fill Mismatch"  
    );  

    // 执行 swap 操作
    (uint256 amountOut, uint256 tokenInRefund) = execute(  
        target,  
        txData,  
        order  
    );  

    // 移除 pendingOrderId
    pendingOrderIds = ArrayMutation.removeFromArray(  
        pendingOrderIdx,  
        pendingOrderIds  
    );
}

function execute(  
    address target,  
    bytes calldata txData,  
    Order memory order  
) internal returns (uint256 amountOut, uint256 tokenInRefund) {  
    // 获取代币初始余额
    uint256 initialTokenIn = order.tokenIn.balanceOf(address(this));  
    uint256 initialTokenOut = order.tokenOut.balanceOf(address(this)); // @audit - 攻击者可在此处控制交易  
}

攻击的路径：

攻击者创建一个恶意代币，其 balanceOf 方法被定制为在调用时执行恶意逻辑（如再次调用 fillOrder）。
攻击者创建一个 1 WETH -> 恶意代币的订单。
再创建另一个任意订单（仅用作占位）。
攻击者调用 fillOrder 填充其订单。在 execute 函数调用 tokenOut.balanceOf 时，恶意代币的逻辑被触发。
在 balanceOf 的恶意逻辑中，攻击者再次调用 fillOrder，填充相同的订单（此时订单尚未从 pendingOrderIds 中移除，重入得以成功）。
两次调用共消耗 2 ETH，但攻击者只需要支付 1 ETH，利用合约中的其余资金完成交易。
攻击者移除其两个订单，重复攻击，直到耗尽合约内的资金。

个人认为这是一个非常好的示例，恶意的代币，那么它实现的ERC20的基础功能就可能会掺入恶意代币，比如这次的balanceOf函数中，就完全可以塞入恶意代码，看来我还得再如了解一下ERC20的基础功能了（苦笑）

计算orderId 可以被预测，然后再覆盖（高）

使用

uint256 orderId = uint256(keccak256(abi.encodePacked(recipient, block.timestamp)));

此方法存在以下缺陷：

缺乏唯一性：当同一块中存在多个 createOrder 调用且具有相同的 recipient 时，这些订单的 orderId 会相同，覆盖订单映射中的条目。
排序器权限问题：在 Optimism 等链上，恶意排序器可以通过重排交易在同一块中插入恶意交易，从而覆盖合法用户的订单。
非标准代币漏洞：攻击者可以利用非标准代币，这些代币的 transferFrom 方法未验证 from 地址，从而进一步强化攻击。

攻击路径

用户在一个区块内提交多个 createOrder 交易，订单的 recipient 相同但订单内容不同。
恶意排序器观察到这些交易后，构造一个利用非标准代币的恶意交易，该代币绕过了 transferFrom 的验证。
恶意排序器将自己的交易插入到区块的最后一笔，对应相同的 recipient。
因为 orderId 是基于 recipient 和 block.timestamp 生成的，前面的订单会被覆盖。
最终，orders[orderId] 映射的值被覆盖为恶意排序器的订单，导致合法用户的资金被替换。

个人认为：这个地方是我之前注意到的，但是没有取仔细的推敲，导致就不能联想到这个正常的交易可以被覆盖，还是时间不充足吧

在同一个区块可以进行多笔的交易，这样他们的时间戳就是一样的
交易被打包上链的过程：

用户发起交易。
信息包括：

交易发起者地址 (from)。
接收者地址 (to)。
发送的金额或调用合约的函数和参数。
用户设置的交易手续费（gasPrice 和 gasLimit）。
随机数 (nonce)，确保交易唯一性。
签名，证明交易是由用户发起的。

广播到网络
验证内容包括：

签名是否有效。
交易 nonce 是否正确（用户账户的 nonce 必须递增）。
用户账户余额是否足够支付交易金额和手续费。
验证通过的交易会被加入到节点的交易池中，等待矿工打包。

交易进入内存池（Mempool）
每个节点都有一个交易内存池（Mempool），存储尚未被打包的交易。
矿工会从交易池中挑选交易，通常优先选择手续费较高的交易，因为矿工的收益来源于手续费。
在 Optimism 等 Layer 2 网络中，交易进入 Sequencer，Sequencer 按一定规则排序并生成候选区块。
矿工打包交易
矿工（或验证者）从交易池中挑选一组交易，进行以下操作：
检查交易：
再次验证交易的合法性，包括签名、账户余额、nonce 等。
确保交易能够成功执行，否则会拒绝。
执行交易：
按照交易的顺序逐一执行，更新区块链的状态（如账户余额、合约存储等）。
执行后，计算交易消耗的 gas 并记录。
创建区块：
将所有成功的交易、区块头、block.timestamp、上一区块哈希等信息打包成一个新区块。
区块广播
矿工完成区块后，将新区块广播到整个网络，所有节点都会验证区块的合法性：
区块头是否正确（包括哈希计算、block.timestamp 等）。
区块中的所有交易是否合法且执行结果一致。
交易确认
当新区块被添加到区块链后：
交易正式被写入区块链，这一过程称为“上链”。
随着后续区块的增加，交易所在区块的深度增加，确认数越高，交易被篡改的可能性越低。

注意点：
交易时间戳 (block.timestamp)：
由矿工决定，通常表示区块打包的时间，而不是每笔交易的时间。
同一个区块内的所有交易共享相同的 block.timestamp。
矿工选择交易的策略：
优先选择 gasPrice 较高的交易。
如果区块空间有限（gasLimit 达到上限），低手续费交易可能被延迟打包。
交易失败的处理：
如果交易执行失败（例如超出 gasLimit 或合约逻辑失败），交易仍会被记录在区块链中，但用户支付的 gas 无法退回。
Layer 2 的不同机制：
在 Rollup（如 Optimism）中，Sequencer 负责快速排序交易并生成区块，然后将区块数据提交到主链。

攻击者可以修改被取消的订单（高）

当订单被取消时，资金会退回给用户。然而，问题在于即使订单已取消，攻击者仍可以修改这些取消的订单，从而获取额外的资金。

//ensure delta is valid  
require(_amountInDelta < order.amountIn, "invalid delta");  

//set new amountIn for accounting  
newAmountIn -= _amountInDelta;  

//check min order size for new amount  
MASTER.checkMinOrderSize(order.tokenIn, newAmountIn);  

//refund position partially  
order.tokenIn.safeTransfer(order.recipient, _amountInDelta);

原因是：
orders[orderId] 在订单取消后未被删除，导致数据仍然保留，并可被进一步修改利用。

个人认为：这个是最多人发现的漏洞，但是如果不敏感，还是发现不了，因为你还是会觉得比较正常，如果是我的话，我也会跳过的，可能经验不足的太明显了

总结一下

还是错过了一个本来能发现的点，那个时间戳的问题，还是的联想其他合约代码是怎么使用的，对于业务逻辑理解不好，我觉得还是的花时间来去正真的看，不然就会使一味的去学习其他人的审计报告。还是好遗憾，那个时间戳的问题，还是没有去发现何实现，唉

一

Summary

Internal pre-conditions

External pre-conditions

Attack Path

Impact

PoC

Mitigation

重入攻击 （高）(第一次遇到在balanceOf填充恶意代码)

计算orderId 可以被预测，然后再覆盖 （高）

攻击路径

攻击者可以修改被取消的订单（ 高）

总结一下

重入攻击（高）(第一次遇到在balanceOf填充恶意代码)

计算orderId 可以被预测，然后再覆盖（高）

攻击者可以修改被取消的订单（高）