Incorrect state update on settlement failure
Description
The _settleOrder function settles a single order and updates balances. When balance updates fail, it skips settling the current order by returning early.
function _settleOrder(bytes32 orderId, address filler) internal {
// [...]
bool successLock = balances[order.offerer][order.inputToken].decreaseLockedNoRevert(
uint128(order.inputAmount)
);
bool successUnlock = balances[filler][order.inputToken].increaseUnlockedNoRevert(
uint128(order.inputAmount)
);
if (!successLock || !successUnlock) {
return; // Any reverts are skipped
}
// [...]
}However, an incorrect balance state could occur in this scenario — if successLock is false but successUnlock is true, the function returns after increasing the filler's unlocked balance, creating an inconsistent state where
the filler's unlocked amount increases,
the offerer's locked amount remains unchanged, and
the order status remains unchanged.
Impact
Incorrect state updates on settlement failure in the _settleOrder function could result in inconsistent balance accounting.
Recommendations
We recommend restoring the original balances when either operation fails:
function _settleOrder(bytes32 orderId, address filler) internal {
if (orderStatus[orderId] != IAori.OrderStatus.Active) {
return; // Any reverts are skipped
}
// Update balances: move from locked to unlocked
Order memory order = orders[orderId];
+ Balance memory offererBalanceCache = balances[order.offerer][order.inputToken];
+ Balance memory fillerBalanceCache = balances[filler][order.inputToken];
bool successLock = balances[order.offerer][order.inputToken].decreaseLockedNoRevert(
uint128(order.inputAmount)
);
bool successUnlock = balances[filler][order.inputToken].increaseUnlockedNoRevert(
uint128(order.inputAmount)
);
if (!successLock || !successUnlock) {
+ balances[order.offerer][order.inputToken] = offererBalanceCache;
+ balances[filler][order.inputToken] = fillerBalanceCache;
return; // Any reverts are skipped
}
orderStatus[orderId] = IAori.OrderStatus.Settled;
emit Settle(orderId);
}Remediation
This issue has been acknowledged by Aori, and fixes were implemented in the following commits: