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

0 stars 0 forks source link

RToken Upgrade Vulnerability #194

Closed c4-bot-7 closed 1 month ago

c4-bot-7 commented 1 month ago

Lines of code

https://github.com/code-423n4/2024-07-reserve/blob/3f133997e186465f4904553b0f8e86ecb7bbacbf/contracts/p1/Main.sol#L117

Vulnerability details

The MainP1 contract's RToken upgrade process has a flaw where can bypass the version check, enabling them to upgrade the RToken component without updating the version.

Root Cause

The version check performed in the upgradeRTokenTo function of the MainP1 contract (Main.sol:111-150). Specifically: Main.sol#L117

require(keccak256(abi.encodePacked(this.version())) == versionHash, "upgrade main first");

This line checks if the current version of the main contract matches the provided versionHash. If it doesn't match, the upgrade process is halted. However, if the versionHash passed to the function is the same as the current version, the upgrade will proceed without changing the version.

Impact

Proof of Concept

Recommended Mitigation Steps

The following changes should be made to the upgradeRTokenTo function in the MainP1 contract

function upgradeRTokenTo(
    bytes32 versionHash,
    bool preValidation,
    bool postValidation
) external onlyRole(OWNER) {
    require(address(versionRegistry) != address(0), "no registry");
-   require(keccak256(abi.encodePacked(this.version())) == versionHash, "upgrade main first");
+   require(keccak256(abi.encodePacked(this.version())) != versionHash, "version must be different");

    Implementations memory implementation = versionRegistry.getImplementationForVersion(
        versionHash
    );

    // ...
}

Assessed type

Upgradable