048.md

Elegant Arctic Stork

High

Broken Owner Change Implementation Due to Variable Shadowing

Summary

Variable shadowing in the owner reassignment function ( changeOwner )will cause a complete failure of ownership transfer for the protocol as any caller attempting to change ownership will result in an ineffective state change.

Root Cause

In AuctionFactory.sol:218 the function parameter owner shadows the state variable owner, causing the assignment to modify the parameter instead of the state variable:

https://github.com/sherlock-audit/2024-11-debita-finance-v3/blob/main/Debita-V3-Contracts/contracts/auctions/AuctionFactory.sol#L218-L222

Internal pre-conditions

1. Current owner needs to call changeOwner() within 6 hours of contract deployment
2. State variable owner must be initialized (done in constructor)

External pre-conditions

NA

Attack Path

1. Current owner calls changeOwner() with new owner address
2. Function executes successfully but state variable remains unchanged
3. Original owner retains control despite appearing to transfer ownership
4. New intended owner has no access to owner functions

Impact

1. Permanent inability to transfer ownership
2. Risk of protocol being locked if original owner loses access
3. No way to update critical protocol parameters that require owner access
4. Potential need for contract redeployment to fix ownership issues

PoC

function testOwnershipTransferFails() public {
    address newOwner = address(0x123);
    address originalOwner = auctionFactory.owner();
    
    vm.prank(originalOwner);
    auctionFactory.changeOwner(newOwner);
    
    assertEq(auctionFactory.owner(), originalOwner); // Still points to original owner
    assertTrue(auctionFactory.owner() != newOwner); // New owner not set
}

Mitigation

function changeOwner(address _newOwner) public {
    require(msg.sender == owner, "Only owner");
    require(deployedTime + 6 hours > block.timestamp, "6 hours passed");
    require(_newOwner != address(0), "Zero address");
    owner = _newOwner; // Fixed assignment
}

The fix involves:

1. Renaming the parameter to avoid shadowing
2. Adding zero address check
3. Properly assigning the new owner to the state variable