097.md

Powerful Sandstone Vulture

High

Ownership transfer functionality is entirely broken in changeOwner Function

Summary

In the AuctionFactory contract, the changeOwner function is intended to allow the contract owner to transfer ownership within the first 6 hours of contract deployment. However, a variable shadowing issue prevents the function from updating the state variable owner. This creates a critical functional flaw, leaving the owner state variable unchanged despite the function's invocation, and renders ownership transfer impossible.

Vulnerability Detail

The AuctionFactory contract defines a state variable owner that stores the address of the contract's owner. This variable is initialized to the deployer's address in the constructor:

address owner; // owner of the contract
constructor() {
    owner = msg.sender;
    feeAddress = msg.sender;
    deployedTime = block.timestamp;
}

The changeOwner function is designed to enable the current owner to update the owner state variable to a new owner within a time window of 6 hours after deployment. However, the function's implementation contains a shadowing issue:

function changeOwner(address owner) public {
    require(msg.sender == owner, "Only owner");
    require(deployedTime + 6 hours > block.timestamp, "6 hours passed");
    owner = owner; //@audit 
}

The owner parameter in the function signature shadows the state variable owner. As a result:

1. The require(msg.sender == owner) condition checks against the function's parameter owner instead of the state variable, allowing anyone to bypass this check.
2. The statement owner = owner; assigns the parameter owner to itself, leaving the state variable unchanged.

As a result, the owner state variable remains unchanged regardless of the input or logic.

This issue is also present in the buyOrderFactory contract and debitaV3Aggregrator contract.

Impact

Ownership transfer functionality is entirely broken as the changeOwner function does not update the owner state variable.

Tools

Manual Review

Recommendation

To fix this issue:

Rename the parameter in the changeOwner function to avoid shadowing the state variable. Correctly update the owner state variable.

function changeOwner(address newOwner) public {
    require(msg.sender == owner, "Only owner");
    require(deployedTime + 6 hours > block.timestamp, "6 hours passed");
    owner = newOwner; 
}