Silly Mandarin Sidewinder
High
The changeOwner function contains a local variable that shadows the global owner variable, preventing the contract's ownership from being updated. This issue arises because the function parameter owner is mistakenly used in place of the global owner state variable, resulting in the assignment owner = owner; affecting only the local scope. As a result, attempts to change ownership are ineffective, leaving the original owner unchanged.``
In the contracts DebitaV3Aggregator.sol, AuctionFactoryDebita.sol, and BuyOrderFactory.sol, the changeOwner function includes a local variable that inadvertently shadows the global owner variable. As a result, when the contract owner attempts to transfer ownership to a new address, the function updates only the local variable due to Solidity's variable precedence rules, leaving the global owner unchanged. This issue prevents the contract's ownership from being updated as intended.
No response
No response
No response
Let's say you deploy a contract and later decide to transfer ownership to a different address for management or security reasons. Due to the variable shadowing issue in the changeOwner function, the contract will fail to update the global owner variable, even though the function executes. Instead, the local parameter owner is modified, leaving the contract’s ownership unchanged.
As a result, despite the intention to transfer control to a new address, the original owner remains in control, and the new address cannot access owner-only functions. This prevents critical operations, such as upgrades, administrative changes, or responding to security needs. Ultimately, the contract becomes unmanageable, locking you into using the original wallet and exposing the system to operational limitations and potential risks.
Add this test case to BasicDebitaAggregator.t.sol and run it. The test will pass both requirements, but the global owner will not be updated.
address attacker = makeAddr("attacker");
function testAnyoneCanChangeOwner() public {
vm.startPrank(attacker);
DebitaV3AggregatorContract.changeOwner(attacker);
vm.stopPrank();
assert(DebitaV3AggregatorContract.owner() != attacker);
}To resolve this, the function parameter should be renamed (e.g., to newOwner) and correctly assigned to the global owner variable
function changeOwner(address newOwner) public {
require(msg.sender == owner, "Only owner");
require(deployedTime + 6 hours > block.timestamp, "6 hours passed");
owner = newOwner;
}