011.md

Smooth Sapphire Barbel

Medium

DebitaV3Aggregator::changeOwner Function Fails to Update Ownership Due to Parameter Shadowing

Summary

The changeOwner function in DebitaV3Aggregator and AuctionFactory fails to correctly update ownership because the owner parameter shadows the contract’s state variable owner. This causes the require(msg.sender == owner, "Only owner") check to compare msg.sender to the parameter owner, rather than the actual state variable. As a result:

• The function will always revert if the provided address is different from the caller’s address.

• The function does not revert if the caller passes their own address as the new owner, but the transaction becomes a no-op — no state changes are made because the state variable is not updated.

These issues prevent the rightful owner from updating the ownership and allow unauthorized users to call the function without effect.

address public owner;

function changeOwner(address owner) public {
    require(msg.sender == owner, "Only owner");
    // @> comparison against the parameter, not the storage variable
    require(deployedTime + 6 hours > block.timestamp, "6 hours passed");
    owner = owner;
    // @> no-op we are updating the parameter instead of the storage variable
}

Root Cause

In https://github.com/sherlock-audit/2024-11-debita-finance-v3/blob/main/Debita-V3-Contracts/contracts/DebitaV3Aggregator.sol#L682 the owner parameter shadows the storage variable with the same name.

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

1. The rightful owner calls the changeOwner function passing a new owner address.

Impact

These issue prevents the rightful owner from updating the ownership and allows unauthorized users to call the function without effect.

PoC

No response

Mitigation

To avoid the conflict between the parameter and the state variable owner, either rename the parameter to newOwner or eliminate the custom changeOwner function and use OpenZeppelin's Ownable library, which provides a secure and tested way to manage ownership.

Option 1: Rename the parameter:

address public owner;

- function changeOwner(address owner) public {
+ function changeOwner(address newOwner) public {
    require(msg.sender == owner, "Only owner");
    require(deployedTime + 6 hours > block.timestamp, "6 hours passed");
-   owner = owner;
+   owner = newOwner;  // Assign the new owner to the state variable
}