Dria
Swan
NFTHardhat
21,000 USDC
View results
Previous Next
Submission Details
Severity: high
Invalid

Race Condition in Oracle Task Processing Enables Double Execution

0xbrett8571

Summary

Race condition in BuyerAgent.sol allows double processing of oracle tasks, leading to duplicate purchases and state updates.

function updateState() external onlyAuthorized {
// @audit-info State check can be bypassed due to non-atomic operation
uint256 taskId = oracleStateRequests[round];
if (isOracleRequestProcessed[taskId]) {
revert TaskAlreadyProcessed();
}
// @audit-info Critical state mutation before completion flag
bytes memory newState = oracleResult(taskId);
state = newState;
isOracleRequestProcessed[taskId] = true;
}
​
function purchase() external onlyAuthorized {
// @audit-info Same race condition vulnerability
uint256 taskId = oraclePurchaseRequests[round];
if (isOracleRequestProcessed[taskId]) {
revert TaskAlreadyProcessed();
}
// @audit-info Asset purchases execute before task completion
bytes memory output = oracleResult(taskId);
address[] memory assets = abi.decode(output, (address[]));
// Purchase logic...
isOracleRequestProcessed[taskId] = true;
}

This fundamentally breaks the BuyerAgent's core security assumptions

  1. Task processing uniqueness

  2. Spending limits enforcement

  3. State consistency

  4. Asset inventory accuracy

The autonomous nature of the buyer agent makes this particularly dangerous as it could lead to uncontrolled asset purchases and state corruption, directly impacting user funds and protocol stability.

Vulnerability Details

Root Cause:

  • Task completion flags (isOracleRequestProcessed) are set after executing critical logic

  • No atomic operation protection between state check and update

  • Shared state between purchase() and updateState()

contracts/swan/BuyerAgent.sol.updateState# https://github.com/Cyfrin/2024-10-swan-dria/blob/c8686b199daadcef3161980022e12b66a5304f8e/contracts/swan/BuyerAgent.sol#L200-L216

contracts/swan/BuyerAgent.sol.purchase# https://github.com/Cyfrin/2024-10-swan-dria/blob/c8686b199daadcef3161980022e12b66a5304f8e/contracts/swan/BuyerAgent.sol#L222-L256

function updateState() external onlyAuthorized {
// @audit-info Race condition: Task processing state check can be bypassed
(uint256 round,) = _checkRoundPhase(Phase.Withdraw);
​
uint256 taskId = oracleStateRequests[round];
if (isOracleRequestProcessed[taskId]) {
revert TaskAlreadyProcessed();
}
​
bytes memory newState = oracleResult(taskId);
state = newState;
​
// @audit-info State update happens before marking task as processed
isOracleRequestProcessed[taskId] = true;
}
​
function purchase() external onlyAuthorized {
// @audit-info Same race condition vulnerability as updateState()
(uint256 round,) = _checkRoundPhase(Phase.Buy);
​
uint256 taskId = oraclePurchaseRequests[round];
if (isOracleRequestProcessed[taskId]) {
revert TaskAlreadyProcessed();
}
​
bytes memory output = oracleResult(taskId);
address[] memory assets = abi.decode(output, (address[]));
​
// @audit-info Critical purchase logic executes before task completion flag
for (uint256 i = 0; i < assets.length; i++) {
address asset = assets[i];
uint256 price = swan.getListingPrice(asset);
spendings[round] += price;
if (spendings[round] > amountPerRound) {
revert BuyLimitExceeded(spendings[round], amountPerRound);
}
inventory[round].push(asset);
swan.purchase(asset);
}
​
isOracleRequestProcessed[taskId] = true;
}

The issue occurs because

  1. Both functions check isOracleRequestProcessed before processing

  2. If called in quick succession, both could pass the initial check

  3. This could lead to double processing of the same task

Proof of Concept

  1. Oracle task is created with taskId X

  2. purchase() is called with taskId X

  3. Before isOracleRequestProcessed[X] is set to true, updateState() is called

  4. Both functions process the same task

Impact

  • Double execution of oracle tasks

  • Duplicate asset purchases

  • State corruption

Tools Used

Vs

Recommendations

contract BuyerAgent {
+ using ReentrancyGuard for uint256;
+ mapping(uint256 => TaskStatus) private taskStatus;
+ enum TaskStatus { NONE, PROCESSING, COMPLETED }
​
function updateState() external onlyAuthorized nonReentrant {
uint256 taskId = oracleStateRequests[round];
- if (isOracleRequestProcessed[taskId]) {
+ if (taskStatus[taskId] != TaskStatus.NONE) {
revert TaskAlreadyProcessed();
}
+ taskStatus[taskId] = TaskStatus.PROCESSING;
bytes memory newState = oracleResult(taskId);
state = newState;
- isOracleRequestProcessed[taskId] = true;
+ taskStatus[taskId] = TaskStatus.COMPLETED;
}
}
Updates

Lead Judging Commences

inallhonesty Lead Judge
12 months ago
inallhonesty Lead Judge 12 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.