Dria
Swan
NFTHardhat
21,000 USDC
View results
Previous Next
Submission Details
Severity: low
Valid

Ownership transfer grants former Swan contract owner continued `operator` privileges

tigerfrake

Summary

During initialization, the Swan contract assigns both the owner and operator roles to the caller. However, transferring ownership via the transferOwnership() function only updates the contract owner address, leaving the previous owner’s operator privileges intact. The new owner on the other hand therefore is denied this status when in reality he is the acting contract owner.

Vulnerability Details

During the contract’s initialization in Swan contract, the caller is assigned both the owner and operator roles.

function initialize(
---SNIP---
) public initializer {
>> __Ownable_init(msg.sender);
​
---SNIP---
// owner is an operator
>> isOperator[msg.sender] = true;
}

This allows them to execute functions gated by the onlyAuthorized() modifier, which requires the caller to be either the BuyerAgent owner or an operator.

modifier onlyAuthorized() {
// if its not an operator, and is not an owner, it is unauthorized
if (!swan.isOperator(msg.sender) && msg.sender != owner()) {
revert Unauthorized(msg.sender);
}
_;
}

However, when Swan contract ownership is transferred using the inherited transferOwnership() function from OwnableUpgradeable, the operator status will not be revoked from the original contract owner and neither will it be granted to the new owner.

function transferOwnership(address newOwner) public virtual onlyOwner {
if (newOwner == address(0)) {
revert OwnableInvalidOwner(address(0));
}
_transferOwnership(newOwner);
}

This creates a situation where the previous Swan contract owner retains operator access even after ownership is transferred, potentially leading to unauthorized access if that previous owner acts on the retained privileges.

Clarification:

There are two owners I am reffering to here:

  1. The Swan owner (Trusted) - This is the wallet that deploys Swan by default (The one given operator status)

  2. BuyerAgent Owner: A user that created a buyer agent with createBuyer() function in Swan

Now according to Contest Details (Actors), Swan Owner is trusted. However, once he transfers this ownership to a new entity, he is nolonger the owner and as such, should not act on previous privileges.

Impact

This oversight could allow a former owner to interact with functions restricted to BuyerAgent owner or
designated operators. Since the onlyAuthorized() modifier allows access to both operators and the BuyerAgent owner, a previous contract owner retaining operator privileges could invoke critical functions, potentially disrupting expected contract functionality or enabling unintended actions.

Tools Used

Manual Review

Recommendations

Override the transferOwnership() function to correctly revoke operator status from the previous owner and assign it to the new owner. This ensures that the privileges managed by the onlyAuthorized() modifier are aligned with the current Swan contract ownership.

function transferOwnership(address newOwner) public override onlyOwner {
if (newOwner == address(0)) {
revert OwnableInvalidOwner(address(0));
}
​
// @audit Revoke operator status from the current owner
+ isOperator[msg.sender] = false;
​
// Transfer ownership using the parent contract's functionality
_transferOwnership(newOwner);
​
// @audit Assign operator status to the new owner
+ isOperator[newOwner] = true;
}
Updates

Lead Judging Commences

inallhonesty Lead Judge 12 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Ownership transfer doesn't fix the operator status.

Support

FAQs

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