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

5 stars 3 forks source link

Kow - Base fee will be initialised to the wrong amount when `RubiconFeeController` is deployed using a proxy #26

Closed sherlock-admin closed 7 months ago

sherlock-admin commented 7 months ago

Kow

medium

Base fee will be initialised to the wrong amount when RubiconFeeController is deployed using a proxy

Summary

Base fee rate will be initialised to zero instead of 0.01% on the RubiconFeeController proxy.

Vulnerability Detail

The default value for baseFee in RubiconFeeController is set to 10 in a storage field declaration. https://github.com/sherlock-audit/2024-02-rubicon-finance/blob/11cac67919e8a1303b3a3177291b88c0c70bf03b/gladius-contracts-internal/src/fee-controllers/RubiconFeeController.sol#L16-L24

contract RubiconFeeController is
    IProtocolFeeController,
    DSAuth,
    ProxyConstructor
{
    ...
    uint256 public baseFee = 10;

This is equivalent to setting baseFee in the constructor. However, RubiconFeeController is intended to be accessed via a proxy as indicated by inheriting from ProxyConstructor and the proxy cannot call the constructor of the implementation contract. It instead calls initialize (via delegatecall) to set its initial storage values. https://github.com/sherlock-audit/2024-02-rubicon-finance/blob/11cac67919e8a1303b3a3177291b88c0c70bf03b/gladius-contracts-internal/src/fee-controllers/RubiconFeeController.sol#L39-L48

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

        initialized = true;
    }

We can see baseFee is not set here. Consequently, while baseFee = 10 on the implementation contract, baseFee = 0 in the proxy storage.

Impact

This can open up a window of opportunity for fillers to execute orders without paying any fee if unnoticed at deployment.

Code Snippet

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

Tool used

Manual Review

Recommendation

Initialise baseFee to 10 in RubiconFeeController::initialize.

sherlock-admin commented 7 months ago

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

tsvetanovv commented:

Low. In addition the admin can use setBaseFee() and set the fee

0xAadi commented:

Invalid: Admin can reset the baseFee later using setBaseFee