code-423n4 / 2023-10-opendollar-findings

10 stars 7 forks source link

Missing updating `nftRenderer` implementation when `SafeManager` address is updated in the `Vault721` contract #360

Closed c4-submissions closed 1 year ago

c4-submissions commented 1 year ago

Lines of code

https://github.com/open-dollar/od-contracts/blob/f4f0246bb26277249c1d5afe6201d4d9096e52e6/src/contracts/proxies/Vault721.sol#L126-L12

Vulnerability details

Impact

Proof of Concept

Vault721.setSafeManager function

  function setSafeManager(address _safeManager) external onlyGovernor {
    _setSafeManager(_safeManager);
  }

Tools Used

Manual Testing.

Recommended Mitigation Steps

Sync/reflect updated SafeManager address with the NFTRenderer contract.

Assessed type

Context

c4-pre-sort commented 1 year ago

raymondfam marked the issue as low quality report

c4-pre-sort commented 1 year ago

raymondfam marked the issue as duplicate of #37

c4-judge commented 1 year ago

MiloTruck marked the issue as not a duplicate

c4-judge commented 1 year ago

MiloTruck marked the issue as unsatisfactory: Invalid

MiloTruck commented 1 year ago

Governance can call updateNftRenderer() afterwards to sync the NFTRenderer contract with the new SafeManager if needed.