sherlock-audit / 2024-02-rubicon-finance-judging

5 stars 3 forks source link

Tychai0s - BaseGladiusReactor can be hijacked #24

Closed sherlock-admin closed 9 months ago

sherlock-admin commented 9 months ago

Tychai0s

medium

BaseGladiusReactor can be hijacked

Title

Audit Report: Uninitialized Implementation Vulnerability in BaseGladiusReactor.sol

Summary

This report outlines a critical vulnerability identified in the BaseGladiusReactor.sol smart contract related to the initialization process through a proxy contract. The core issue arises from the potential scenario where the deployer initializes the contract through the proxy but neglects to do the same directly on the implementation contract, leaving the implementation vulnerable to unauthorized access and potential hijacking.

Vulnerability Detail

The vulnerability, found in BaseGladiusReactor.sol:40, stems from the contract's design allowing for its initialization via a proxy contract. While this is a common pattern for upgradable contracts, failing to also initialize the implementation directly can lead to a scenario where the implementation remains uninitialized. This oversight creates an opportunity for attackers to initialize the implementation contract themselves, potentially taking control of the logic contract in an unauthorized manner.

Impact

If the implementation contract remains uninitialized, it poses a significant security risk. An attacker could exploit this oversight by calling the initialize function on the implementation, setting themselves as the owner or manipulating critical contract parameters.

Code Snippet

    function initialize(address _permit2, address _owner) external override {
        if (initialized) revert AlreadyInitialized();
        permit2 = IPermit2(_permit2);
        owner = _owner;

        initialized = true;
    }

Tool used

Manual Review

Recommendation

It is recommended to ensure that the implementation contract is also properly initialized in a manner that prevents unauthorized subsequent initializations. This can be achieved by using a constructor in the implementation contract that straightforward marks the contract as already initialized.

// BaseGladiusReactor.sol
// ...
constructor() {
    initialized = true;
}
// ...
sherlock-admin commented 9 months ago

1 comment(s) were left on this issue during the judging contest.

0xAadi commented: