0xDemon - Initialize function can be front run by attacker

[sherlock-admin]

0xDemon

medium

Initialize function can be front run by attacker

Initialize function can be front run by attacker

Summary

The protocol initializes the contract using the initialize function where this function does not apply access control to prevent malicious actors from front running it. There are two examples of this case, one of which is RubiconFeeController::initialize, the purpose of this initialization is to set the owner of the contract and feeRecipient. The affected code :

File : gladius-contracts-internal/src/fee-controllers/RubiconFeeController.sol

function initialize(
        address _owner,
        address _feeRecipient
    ) external override {
        if (initialized) revert AlreadyInitialized();
        owner = _owner;
        feeRecipient = _feeRecipient;

        initialized = true;
    }

Some consequences if this happens:

1. The attacker becomes the owner of the contract and can also be the feeRecipient of the protocol
2. Because now attacker become the owner of contract, he/she can call and set the value of setPairBasedFee, setBaseFee, setFeeRecipient, and setGladiusReactor functions.
3. If the true owner of the contract want to initialized, it will be revert because it can be only initialized once

Vulnerability Detail

POC

To prove this attack vector, I used the existing test environment with slight changes to the setup function

function setUp() public {
    feeController = new RubiconFeeController();
}

function test_frontruninitialize() public {
        address Alice = makeAddr("Alice");
        vm.prank(Alice);
    //Alice front running the initialize step, set her own as owner and feeRecipient
        feeController.initialize(Alice,Alice);
        assertEq(feeController.owner(), Alice);
        assertEq(feeController.feeRecipient(), Alice);

        //revert for intialized for a second time
        vm.prank(PROTOCOL_FEE_OWNER);
        vm.expectRevert();
        feeController.initialize(PROTOCOL_FEE_OWNER, RECIPIENT);
    }

To execute it, just insert this test into RubiconFeeController.t.sol and in the setUp function just deploy RubiconFeeController. After that execute with :

forge test --match-test test_frontruninitialize

Result :

Running 1 test for test/fee-controllers/RubiconFeeController.t.sol:RubiconFeeControllerTest
[PASS] test_frontruninitialize() (gas: 57986)
Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 44.58ms

Impact

1. The attacker becomes the owner of the contract and can also be the feeRecipient of the protocol
2. Because now attacker become the owner of contract, he/she can call and set the value of setPairBasedFee, setBaseFee, setFeeRecipient, and setGladiusReactor functions.
3. If the true owner of the contract want to initialized, it will be revert because it can be only initialized once

Code Snippet

https://github.com/sherlock-audit/2024-02-rubicon-finance/blob/main/gladius-contracts-internal/src/fee-controllers/RubiconFeeController.sol#L39-L48

https://github.com/sherlock-audit/2024-02-rubicon-finance/blob/main/gladius-contracts-internal/src/reactors/BaseGladiusReactor.sol#L40-L46

Tool used

Manual review

Recommendation

Add a require statement to each initialize function to verify that the function is called by the contract owner only, and post verification roles should be setup. Otherwise, setting the owner in the contract’s constructor to the msg.sender and adding the onlyOwner modifier to all initializers would be enough for access control.

[sherlock-admin]

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

tsvetanovv commented:

Low

PNS commented:

Front-running initializers where there is no irreversible damage or loss of funds & the protocol could just redeploy and initialize again is not a valid issue.

0xAadi commented:

Invalid