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

Access Control Bypass in BuyerAgent Oracle Integration

aariiif

Summary

The BuyerAgent contract's oracle integration system allows unauthorized state manipulation through improper access control in oracle request processing. The vulnerability stems from missing validation between oracle task requests and their processing, allowing potential manipulation of purchase decisions and state updates.

function oracleResult: https://github.com/Cyfrin/2024-10-swan-dria/blob/c8686b199daadcef3161980022e12b66a5304f8e/contracts/swan/BuyerAgent.sol#L158-L165

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

// @Issue - No validation of oracle request ownership
function oracleResult(uint256 taskId) public view returns (bytes memory) {
if (taskId == 0) {
revert TaskNotRequested();
}
return swan.coordinator().getBestResponse(taskId).output;
}
​
// @Issue - Lack of request-process binding
function purchase() external onlyAuthorized {
(uint256 round,) = _checkRoundPhase(Phase.Buy);
uint256 taskId = oraclePurchaseRequests[round];
if (isOracleRequestProcessed[taskId]) {
revert TaskAlreadyProcessed();
}
// Processes potentially manipulated oracle output
bytes memory output = oracleResult(taskId);
address[] memory assets = abi.decode(output, (address[]));

Vulnerability Details

The core issue lies in how oracle requests are processed and validated.

The onlyAuthorized modifier implementation in BuyerAgent.sol: https://github.com/Cyfrin/2024-10-swan-dria/blob/c8686b199daadcef3161980022e12b66a5304f8e/contracts/swan/BuyerAgent.sol#L105-L111

modifier onlyAuthorized() {
// @Issue - Access control vulnerability: Swan contract access relies on operator status
// Swan should have direct privileged access regardless of operator status
// Current implementation could block Swan if operator status changes
if (!swan.isOperator(msg.sender) && msg.sender != owner()) {
revert Unauthorized(msg.sender);
}
_;
}

  1. The onlyAuthorized modifier checks:

    • If sender is an operator via swan.isOperator()

    • If sender is the owner

  2. The bug occurs because Swan's address is checked through isOperator() rather than having a dedicated check for the Swan contract address itself

  3. This creates a scenario where Swan could lose operator status but still need direct access to these critical functions

The access control logic that governs critical functions, purchase() and updateState(). The current implementation creates a dependency between Swan's access rights and its operator status, which violates the protocol's core security assumptions.

Key risks:

  1. Protocol functionality can break if Swan loses operator status

  2. Core market operations could be interrupted

  3. Violates principle of least privilege by coupling different access control mechanisms

Consider this POC

function testSwanAccessLockout() public {
// Setup
address swanAddress = address(swan);
// Initial state - Swan can access functions
vm.prank(swanAddress);
buyerAgent.purchase(); // succeeds
// Remove operator status
swan.revokeOperator(swanAddress);
// Swan now locked out of core functionality
vm.prank(swanAddress);
vm.expectRevert("Unauthorized");
buyerAgent.purchase();
}

Impact

  • Unauthorized parties can influence purchase decisions

  • State manipulation across trading rounds

Recommendations

modifier onlyAuthorized() {
- if (!swan.isOperator(msg.sender) && msg.sender != owner()) {
+ if (msg.sender != address(swan) && !swan.isOperator(msg.sender) && msg.sender != owner()) {
revert Unauthorized(msg.sender);
}
_;
}

This ensures Swan maintains direct access regardless of operator status, properly separating protocol-level and administrative access controls.

function oracleResult(uint256 taskId) public view returns (bytes memory) {
if (taskId == 0) {
revert TaskNotRequested();
}
+ // Validate task ownership
+ if (!swan.coordinator().isTaskOwner(taskId, address(this))) {
+ revert UnauthorizedTask();
+ }
return swan.coordinator().getBestResponse(taskId).output;
}
​
function purchase() external onlyAuthorized {
(uint256 round,) = _checkRoundPhase(Phase.Buy);
uint256 taskId = oraclePurchaseRequests[round];
+ // Validate task request origin
+ require(swan.coordinator().getTaskRequest(taskId).requester == address(this));
if (isOracleRequestProcessed[taskId]) {
revert TaskAlreadyProcessed();
}

These changes ensure proper validation of oracle task ownership and request origins, maintaining the integrity of the autonomous buying system.

Updates

Lead Judging Commences

inallhonesty Lead Judge 12 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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