Upgraded Q -> 2 from #165 [1699030231989]

[c4-judge]

Judge has assessed an item in Issue #165 as 2 risk. The relevant finding follows:

Clear safeCan in transferSAFEOwnership Links to affected code https://github.com/open-dollar/od-contracts/blob/f4f0246bb26277249c1d5afe6201d4d9096e52e6/src/contracts/proxies/ODSafeManager.sol#L136

Impact Old approval remains even if user gets SAFE again.

Proof of Concept There is no removal safeCan at transferSAFEOwnership . When the user gets SAFE again, the old approval is still remained.

modifier safeAllowed(uint256 _safe) { address _owner = _safeData[_safe].owner; if (msg.sender != _owner && safeCan[_owner][safe][msg.sender] == 0) revert SafeNotAllowed(); ; }

function transferSAFEOwnership(uint256 _safe, address _dst) external { require(msg.sender == address(vault721), 'SafeMngr: Only Vault721');

if (_dst == address(0)) revert ZeroAddress();
SAFEData memory _sData = _safeData[_safe];
if (_dst == _sData.owner) revert AlreadySafeOwner();

_usrSafes[_sData.owner].remove(_safe);
_usrSafesPerCollat[_sData.owner][_sData.collateralType].remove(_safe);

_usrSafes[_dst].add(_safe);
_usrSafesPerCollat[_dst][_sData.collateralType].add(_safe);

_safeData[_safe].owner = _dst;

emit TransferSAFEOwnership(msg.sender, _safe, _dst);

} Tools Used Manual Review

Recommended Mitigation Steps Delete safeCan at transferSAFEOwnership . You might use Set instead of mapping.

[c4-judge]

MiloTruck marked the issue as duplicate of #142

[c4-judge]

MiloTruck marked the issue as satisfactory