0x52 - Contracts never initialize owner for Ownable2Step.sol

[sherlock-admin]

0x52

medium

Contracts never initialize owner for Ownable2Step.sol

Summary

OZ's Ownable2Step.sol now requires the owner to be explicitly set rather than assuming msg.sender as before. Since the contracts never call the Ownable.sol contructor _owner is never set and is left as address(0). This makes PriceOracle.sol completely nonfunctional and disables the critical seize and seizeNative functions for both TxBuilderExtention.sol and UniswapExtension.sol.

Vulnerability Detail

Ownable.sol#L28-L30

constructor(address initialOwner) {
    _transferOwnership(initialOwner);
}

The Ownable.sol (inherited by Ownable2Step.sol) constructor now requires the owner to be explicitly set rather than always defaulting to msg.sender.

Impact

PriceOracle.sol completely is nonfunctional and seize and seizeNative functions for both TxBuilderExtention.sol and UniswapExtension.sol are disabled.

Code Snippet

PriceOracle.sol#L30-L34

TxBuilderExtension.sol#L82-L87

UniswapExtension.sol#L127-L141

Tool used

Manual Review

Recommendation

Call the Ownable.sol constructor to set the owner

[ibsunhub]

The version of OZ's Ownerable2Step we used is v4.8.1. It would pass the msg.sender to initialize the owner.

https://github.com/OpenZeppelin/openzeppelin-contracts/blob/v4.8.1/contracts/access/Ownable.sol#L29

[ibsunhub]

The .gitmodules file was not copied to here? No wonder why they keep asking what's the version of OZ we used.

[0xffff11]

Invalid issue because it implements v4.8.1 of OpenZeppelin, which it directly initializes with msg.sender, making the watson's issue invalid