041.md

Festive Fuchsia Shell

High

Unable to change owner of contracts due to shadowed variable declaration

Summary

There are a few contracts that implement a function to change the owner. Because of how the owner variable is declared, the function will always revert when trying to change the owner address.

Root Cause

The root cause is in how the variable owner is passed into the changeOwner function. It shadows the already initialized owner variable in the constructor.

 constructor(address _implementationContract) {
        owner = msg.sender;
        feeAddress = msg.sender;
        implementationContract = _implementationContract;
        deployedTime = block.timestamp;
    }

// change owner of the contract only between 0 and 6 hours after deployment
    function changeOwner(address owner) public {
        require(msg.sender == owner, "Only owner");
        require(deployedTime + 6 hours > block.timestamp, "6 hours passed");
        owner = owner;
    }

Therefore whenever this function is called, it is going to check msg.sender against the new intended owner rather than the existing owner. The following contracts all have the same issue for this function

• AuctionFactory.sol
• DebitaV3Aggregator.sol
• buyOrderFactory.sol

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

Current owner attempts to call changeOwner function

Impact

Unable to change the owner of certain contracts

PoC

Can add a similar test to any of the contracts and it will revert when trying to change the address to any owner

function testChangeFactoryOwner() public {
        address newOwner = makeAddr("Bob");
        vm.prank(address(factory.owner()));
        vm.expectRevert("Only owner");
        factory.changeOwner(address(newOwner));
    }

Mitigation

Fix the owner parameter

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