089.md

Rich Carmine Seagull

Medium

Using Non-Upgradeable ReentrancyGuard in Upgradeable Contractarty

Summary

Using non-upgradeable ReentrancyGuard in an upgradeable contract can cause storage collisions and reentrancy vulnerabilities, potentially allowing attackers to drain funds through reentrancy attacks.

https://github.com/sherlock-audit/2024-12-ethos-update/blob/c3a2b007d0ddfcb476f300f8b766808f0e3e2dfd/ethos/packages/contracts/contracts/ReputationMarket.sol#L8

Root Cause

in ReputationMarket.sol the contract inherits from non-upgradeable ReentrancyGuard while being upgradeable:

contract ReputationMarket is AccessControl, UUPSUpgradeable, ReentrancyGuard, ITargetStatus {

Non-upgradeable ReentrancyGuard uses fixed storage slot that conflicts with proxy pattern:

// ReentrancyGuard (non-upgradeable)
uint256 private _status; // Fixed slot 0

// vs ReentrancyGuardUpgradeable 
bytes32 private constant ReentrancyGuardStorageLocation; // Namespaced slot

Internal Pre-conditions

No response

External Pre-conditions

No response

Attack Path

No response

Impact

• Storage collisions between ReentrancyGuard._status and proxy storage
• Potential reentrancy vulnerabilities in those functions buyVotes(), sellVotes(), withdrawDonations()

PoC

No response

Mitigation

use ReentrancyGuardUpgradeable instead

contract ReputationMarket is AccessControl, UUPSUpgradeable, ReentrancyGuardUpgradeable, ITargetStatus {
    function initialize(
        address owner,
        address admin,
        address expectedSigner,
        address signatureVerifier,
        address contractAddressManagerAddr
    ) external initializer {
        __ReentrancyGuard_init();
        __accessControl_init(
            owner,
            admin,
            expectedSigner,
            signatureVerifier,
            contractAddressManagerAddr
        );
        __UUPSUpgradeable_init();
        // ...existing code...
    }
}