Dria

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

Authorization Bypass in BuyerAgent Through Malicious Swan Contract

Summary

BuyerAgent contract's authorization system can be completely bypassed by deploying it with a malicious Swan contract implementation. This allows unauthorized access to all privileged functions including treasury management, purchasing decisions, and state updates.

The vulnerability exists in the constructor's unchecked initialization of the Swan contract: https://github.com/Cyfrin/2024-10-swan-dria/blob/c8686b199daadcef3161980022e12b66a5304f8e/contracts/swan/BuyerAgent.sol#L120-L144

constructor(
string memory _name,
string memory _description,
uint96 _royaltyFee,
uint256 _amountPerRound,
address _operator,
address _owner
) Ownable(_owner) {
// @Issue - No validation of Swan contract legitimacy
swan = Swan(_operator);
// @Issue - Unchecked token approvals to potentially malicious addresses
swan.token().approve(address(swan.coordinator()), type(uint256).max);
swan.token().approve(address(swan), type(uint256).max);
}

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

modifier onlyAuthorized() {
// @Issue - Relies on potentially compromised Swan.isOperator()
if (!swan.isOperator(msg.sender) && msg.sender != owner()) {
revert Unauthorized(msg.sender);
}
_;
}

Vulnerability Details

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

modifier onlyAuthorized() {
// @Issue - Relies on potentially compromised Swan.isOperator()
if (!swan.isOperator(msg.sender) && msg.sender != owner()) {
revert Unauthorized(msg.sender);
}
_;
}

The Bug: The access control check relies on two conditions:

  • swan.isOperator(msg.sender)

  • msg.sender != owner()

However, there's a flaw in the initialization sequence where the Swan contract address is set in the constructor but there's no validation that it's a legitimate Swan contract implementation.

constructor(
string memory _name,
string memory _description,
uint96 _royaltyFee,
uint256 _amountPerRound,
address _operator,
address _owner
) Ownable(_owner) {
// @Issue - No validation of _operator address being a legitimate Swan contract
// @Issue - Missing checks for zero address inputs
// @Issue - No verification of Swan interface implementation
swan = Swan(_operator);
  1. Missing Contract Validation:

  • The constructor blindly trusts the provided _operator address is a legitimate Swan contract

  • An attacker could deploy a malicious contract implementing the Swan interface

  • This would allow bypassing access controls through a compromised isOperator() check

  1. Security Implications:

  • Full control over buyer agent operations

  • Ability to manipulate purchases and state updates

  • Access to treasury funds through withdrawal functions

  • Potential to lock or drain user funds

Attack vector:

// Malicious Swan Implementation
contract MaliciousSwan {
function isOperator(address) public pure returns (bool) {
return true; // Always returns true
}
}

It's all because the constructor blindly trusts the provided _operator address and casts it to Swan without verification:

swan = Swan(_operator);

This allows an attacker to deploy a malicious contract that always returns true for isOperator() checks, completely bypassing the authorization system.

constructor(
string memory _name,
string memory _description,
uint96 _royaltyFee,
uint256 _amountPerRound,
address _operator, // @Issue - No validation of operator address
address _owner
) Ownable(_owner) {
// @Issue - Blindly trusts operator is legitimate Swan contract
swan = Swan(_operator);
// @Issue - Missing interface verification
amountPerRound = _amountPerRound;

Impact

  • Unauthorized access to privileged functions

  • Potential manipulation of buyer agent state

  • Ability to execute unauthorized purchases

  • Control over treasury withdrawals

Recommendations

constructor(
string memory _name,
string memory _description,
uint96 _royaltyFee,
uint256 _amountPerRound,
address _operator,
address _owner
) Ownable(_owner) {
+ require(_operator != address(0), "Zero address operator");
+ require(Address.isContract(_operator), "Operator must be contract");
swan = Swan(_operator);
+ require(swan.supportsInterface(type(ISwan).interfaceId), "Invalid Swan");
+
+ address coordinator = address(swan.coordinator());
+ address token = address(swan.token());
+ require(coordinator != address(0) && token != address(0), "Invalid addresses");
+ require(IERC20(token).totalSupply() > 0, "Invalid token");
// Approve after validation
IERC20(token).approve(coordinator, type(uint256).max);
IERC20(token).approve(_operator, type(uint256).max);
}
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.