Open Dollar - HChang26's results

Lines of code

https://github.com/open-dollar/od-contracts/blob/v1.5.5-audit/src/contracts/proxies/ODSafeManager.sol#L105 https://github.com/open-dollar/od-contracts/blob/v1.5.5-audit/src/contracts/proxies/ODSafeManager.sol#L49

Vulnerability details

Impact

Authorized users can delegate the allowSAFE() function without requiring owner authorization.

Proof of Concept

The allowSAFE() function permits the owner of a safe to grant permission to another user to manage their safe. Access control is managed through the safeAllowed modifier, ensuring that only authorized users can perform this action. However, a potential issue arises where authorized users can delegate this power to other users, allowing them to use allowSAFE() without the requirement for owner authorization. The original intention was for the owner of the safe to be the sole entity capable of granting this permission. This issue allows for unauthorized individual to make changes to the safe.

  function allowSAFE(uint256 _safe, address _usr, uint256 _ok) external safeAllowed(_safe) {
    address _owner = _safeData[_safe].owner;
    safeCan[_owner][_safe][_usr] = _ok;
    emit AllowSAFE(msg.sender, _safe, _usr, _ok);
  }

Tools Used

Manual Review

Recommended Mitigation Steps

  function allowSAFE(uint256 _safe, address _usr, uint256 _ok) external safeAllowed(_safe) {
    address _owner = _safeData[_safe].owner;
+   require(msg.sender == _owner);
    safeCan[_owner][_safe][_usr] = _ok;
    emit AllowSAFE(msg.sender, _safe, _usr, _ok);
  }

Assessed type

Access Control