My UUPS got hacked - how?!

Security
,
1

Hi there,
I have been using a UUPS pattern since over a year and never had any issues.
Last Saturday morning however, my smart contract 0x7F15F01af90Fd8c1AA32264f44DB02E6bEd259aF on Polygon Mainnet appears to got hacked.

Here is my contract code:

contract TradeController is
    Initializable, 
    UUPSUpgradeable, 
    ERC2771ContextUpgradeable {
   
		/// @custom:oz-upgrades-unsafe-allow constructor
    constructor(address forwarder) ERC2771ContextUpgradeable(forwarder) {}

    function initialize(address registry) public initializer {
        __UUPSUpgradeable_init();
        _registryAddress = registry;
        _feeCollector = payable(_msgSender());
        interfaceId = type(ITradeController).interfaceId;
    }
    function _authorizeUpgrade(address newImplementation) internal override onlyProxy{}
    
    (...)
 }

This is how the attacked managed to Initialize the contract again and set ownership to a new address (not sure how it was 0x before, I swear it was set to my deployer address)

The attacker called a method on his helper contract (0xDDDEaA5dB4523bD714d60Cd301f5Bc5fE485aa8D) and this resulted in the hack:
https://polygonscan.com/vmtrace?txhash=0xdfa321e7b24c8daa35069cc909ac224458185950b056652c76405820c1dd1895&type=gethtrace2
which lead to the following LOGS

Note that the attacker never directly interacted with the smart contract, but used some smart contract which he called my contract from.
My best guess is that the attacker used a function where call data can be passed, which is then forwarded to another account - since the _authorizeUpgrade() override only checks if the call is coming from the proxy contract, it would be sufficient to pass the authorization.

Security functions that apparently failed me:
My existing proxy was apparently initialized again ?! - but shouldn't the inheritance from Initializable contract have prevented exactly this?!

I had also set up a OZ defender v2 monitor, but no alarm whatsoever can was recognized :slightly_frowning_face:

Any idea what the security hole could have been and how to prevent it in the future?

2

The _authorizeUpgrade function in your implementation did not use any access control, which meant that anyone could call the upgradeToAndCall(address, bytes) function of your UUPS proxy to upgrade it to an arbitrary implementation.

As documented in UUPSUpgradeable, the _authorizeUpgrade function must be overridden to include access restriction to the upgrade mechanism. For example, this could be done by using an access control modifier such as Ownable.onlyOwner.

From the trace that you provided, it appears that the attacker called a proxy whose implementation is a contract they own (based on the logs). From there, it called upgradeToAndCall(address, bytes) setting a new implementation owned by the attacker.

This is how the attacked managed to Initialize the contract again and set ownership to a new address (not sure how it was 0x before, I swear it was set to my deployer address)

It looks like your original implementation did not use Ownable, but the attacker's new implementation does, which would be why the owner was 0x before. Regardless, since the attacker upgraded your proxy to their own implementation, they could set ownership however they wanted.

My existing proxy was apparently initialized again ?! - but shouldn't the inheritance from Initializable contract have prevented exactly this?!

Initializable does not protect against this case. The attacker could craft their new implementation with arbitrary logic so they can get around any previous initializations. Hence it is important to properly restrict access to your proxy's upgrade mechanism.

3 Likes
3

thank you for the fast response!
I am just glad that I set onlyRole(UPGRADER_ROLE) for the _authorizeUpgrade function on all my other contracts. Just double checked. Will never again take this lightly.

I assume this contract can't be recovered in any way since the attacker set himself as owner and implemented their own logic. Already started a migration, it was only a beta-test fortunately.

4

Hi @ratio if you could provide us with your account's tenantId we can take a look into your Defender Monitor configuration and determine why it wasn't triggered when this event occurred.