code-423n4 / 2024-07-reserve-validation

0 stars 0 forks source link

Misallocation of reserved space, which will lead to a storage collision when updating the contracts concerned. #32

Closed c4-bot-4 closed 1 month ago

c4-bot-4 commented 2 months ago

Lines of code

https://github.com/code-423n4/2024-07-reserve/blob/3f133997e186465f4904553b0f8e86ecb7bbacbf/contracts/mixins/Auth.sol#L225 https://github.com/code-423n4/2024-07-reserve/blob/3f133997e186465f4904553b0f8e86ecb7bbacbf/contracts/mixins/ComponentRegistry.sol#L118 https://github.com/code-423n4/2024-07-reserve/blob/3f133997e186465f4904553b0f8e86ecb7bbacbf/contracts/p1/AssetRegistry.sol#L273 https://github.com/code-423n4/2024-07-reserve/blob/3f133997e186465f4904553b0f8e86ecb7bbacbf/contracts/p1/BackingManager.sol#L346 https://github.com/code-423n4/2024-07-reserve/blob/3f133997e186465f4904553b0f8e86ecb7bbacbf/contracts/p1/BasketHandler.sol#L784 https://github.com/code-423n4/2024-07-reserve/blob/3f133997e186465f4904553b0f8e86ecb7bbacbf/contracts/p1/Broker.sol#L307 https://github.com/code-423n4/2024-07-reserve/blob/3f133997e186465f4904553b0f8e86ecb7bbacbf/contracts/p1/Distributor.sol#L286 https://github.com/code-423n4/2024-07-reserve/blob/3f133997e186465f4904553b0f8e86ecb7bbacbf/contracts/p1/Furnace.sol#L97 https://github.com/code-423n4/2024-07-reserve/blob/3f133997e186465f4904553b0f8e86ecb7bbacbf/contracts/p1/Main.sol#L170 https://github.com/code-423n4/2024-07-reserve/blob/3f133997e186465f4904553b0f8e86ecb7bbacbf/contracts/p1/RevenueTrader.sol#L204 https://github.com/code-423n4/2024-07-reserve/blob/3f133997e186465f4904553b0f8e86ecb7bbacbf/contracts/p1/RToken.sol#L545 https://github.com/code-423n4/2024-07-reserve/blob/3f133997e186465f4904553b0f8e86ecb7bbacbf/contracts/p1/StRSR.sol#L999 https://github.com/code-423n4/2024-07-reserve/blob/3f133997e186465f4904553b0f8e86ecb7bbacbf/contracts/p1/StRSRVotes.sol#L300 https://github.com/code-423n4/2024-07-reserve/blob/3f133997e186465f4904553b0f8e86ecb7bbacbf/contracts/p1/mixins/Component.sol#L74 https://github.com/code-423n4/2024-07-reserve/blob/3f133997e186465f4904553b0f8e86ecb7bbacbf/contracts/p1/mixins/Trading.sol#L164

Vulnerability details

Sommary

The storage spaces of all contracts whose parent is UUPSUpgradeable misallocates the slots reserved for its storage variables. The top 51 storage slots are used for upgradeable storage.

Impact

Storage collision and breach of logic implemented when updating contracts to new versions.

Proof of Concept

In the UUPSUpgradeable contrat, the first 51 (from 0 to 50) slots have been reserved to allow future versions to add new variables without disrupting the storage in the inheritance chain.So all the storage space for all the contracts that will inherit this contract should start at slot 51.

    abstract contract UUPSUpgradeable is Initializable, IERC1822ProxiableUpgradeable, ERC1967UpgradeUpgradeable {
    // ...

@>  uint256[50] private __gap;

}

However, in the contracts that implement this abstract contract, these 51 reserved slots are not taken into account. We see this for example, with contract DistributorP1 and contract RTokenP1. contract DistributorP1 takes 53 slots so it will start at slot 51 and end at slot 103 and contract RTokenP1 takes 56 slots, so it will start at slot 51 and end at 106.


    contract DistributorP1 is ComponentP1, IDistributor {

    // ...

    /**
     * // ...
     *
@>   * Distributor uses 53 slots, not 50.
     */
@>   uint256[44] private __gap;

}

***********************************************************************

contract RTokenP1 is ComponentP1, ERC20PermitUpgradeable, IRToken {

    // ...

    /**
     * //...
     *
@>   * RToken uses 56 slots, not 50.
     */

@>  uint256[42] private __gap;
}

## Tools Used
Manual Analysis.

## Recommended Mitigation Steps
Bear in mind that the first 51 storage spaces are no longer available as they have been reserved for upgradeable storage.

For example, if we plan 200 slots in total. We know that the first 51 are reserved for upgradeable storage, so there are 149 free slots that the other contracts will share. If a contract takes X out of the remaining 149 slots, there will be 149-X free slots left. it's easy to calculate.

Modify the storage allocation of the contracts listed above as follows and apply the same principle to all the contracts that implement contract `UUPSUpgradeable` 

## Assessed type

Upgradable