Skip to content

Latest commit

 

History

History
70 lines (51 loc) · 2.78 KB

007.md

File metadata and controls

70 lines (51 loc) · 2.78 KB

Cheery Powder Boa

Medium

Variable shadowing in multiple contracts causes changeOwner() to malfunction

Summary

The function changeOwner(address owner) is not working correctly due to a clash between the parameter address owner and the variable address public owner in DebitaV3Aggregator.sol and AuctionFactory.sol and buyOrderFactory.sol. The locally assigned parameter takes precedence, therefore the ownership is not transferred.

Root Cause

In DebitaV3Aggregator.sol the address public owner variable is shadowed by the changeOwner(address owner) function parameter, because both variables share the exact same name.

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

Code locations: address public owner https://github.com/sherlock-audit/2024-11-debita-finance-v3/blob/main/Debita-V3-Contracts/contracts/DebitaV3Aggregator.sol#L198 changeOwner(address owner) https://github.com/sherlock-audit/2024-11-debita-finance-v3/blob/main/Debita-V3-Contracts/contracts/DebitaV3Aggregator.sol#L682

Same issues in AuctionFactory.sol and buyOrderFactory.sol: https://github.com/sherlock-audit/2024-11-debita-finance-v3/blob/main/Debita-V3-Contracts/contracts/auctions/AuctionFactory.sol#L218 https://github.com/sherlock-audit/2024-11-debita-finance-v3/blob/main/Debita-V3-Contracts/contracts/buyOrders/buyOrderFactory.sol#L186

Internal pre-conditions

No preconditions required.

External pre-conditions

No preconditions required.

Attack Path

  1. Owner calls changeOwner(address owner) with intended new owner's address as parameter
  2. Function call fails because the variable shadowing causes the require(msg.sender == owner, ...) check to fail
  3. Even if msg.sender submits his own address to bypass the require statement, the final assignment will do nothing as the owner variable is assigned locally

Impact

Initial owner will not be able to set new owner. The sponsor confirmed in a private thread that they intend to transfer ownership to a multisig after deployment, therefore this action will fail.

PoC

function testChangeOwner() public {
   address originalOwner = DebitaV3AggregatorContract.owner();
   address wannabeOwner = address(0x1234);
   vm.startPrank(originalOwner);
   DebitaV3AggregatorContract.changeOwner(wannabeOwner);
   vm.stopPrank();
   assertEq(DebitaV3AggregatorContract.owner(), originalOwner);
}

Mitigation

function changeOwner(address _owner) public { // change parameter name to _owner
   require(msg.sender == owner, "Only owner"); 
   require(deployedTime + 6 hours > block.timestamp, "6 hours passed");
   owner = _owner; // change right side of the assignment
}