Skip to content

Latest commit

 

History

History
56 lines (34 loc) · 1.64 KB

070.md

File metadata and controls

56 lines (34 loc) · 1.64 KB

Tangy Tortilla Fox

Medium

cntracts sould use ReentrancyGuardUpgradeable as it supports upgradable contracts, unlike ReentrancyGuard

Summary

Currently ReputationMarket is upgradable and possibly planning upgrade in the future. indicators for such behavior are

  1. ReputationMarket using UUPSUpgradeable
  2. The added gap inside both ReputationMarket and it's inherited AccessControl https://github.com/sherlock-audit/2024-12-ethos-update/blob/main/ethos/packages/contracts/contracts/ReputationMarket.sol#L151
  uint256[50] private __gap;
  1. AccessControl being upgradable too

However even though the team intentions are for ReputationMarket to be upgradable it still lacks the necessary stability as it still uses the normal non-upgradable ReentrancyGuard, which has variable in it:

https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/utils/ReentrancyGuard.sol#L37C1-L40C29

    uint256 private constant NOT_ENTERED = 1;
    uint256 private constant ENTERED = 2;

    uint256 private _status;

Root Cause

No response

Internal Pre-conditions

No response

External Pre-conditions

  1. Oz to add a param inside ReentrancyGuard
  2. Admins to upgrade the contracts

Attack Path

No response

Impact

Upgrading such contracts may be dangerous as if OZ (notorious for changing their standard contracts) has added some params to ReentrancyGuard it would cause a storage collision inside ReputationMarket, bricking the contract

PoC

No response

Mitigation

Use ReentrancyGuardUpgradeable instead of ReentrancyGuard in order to make the contracts more secure.