Open Dollar - Bughunter101's results

Lines of code

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

Vulnerability details

Impact

allowSAFE() function dose not check the msg.sender == _safeData[_safe].owner, it may cause the owner loses control Assume the following scenario:

1. Alice is the owner. She calls allowSAFE() to make Bob a safeCan user.
2. Alice wants to cancel Bob’s safeCan permissions
3. However, before Alice calls the cancel function, Bob can add safeCan permission to another account (we assume Bob’s other account is called Car) by calling allowSAFE(), without Alice knowing.
4. Finally, even if Alice cancels Bob's permissions, Bob can still exercise privileges through the Car account

Proof of Concept

  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

Recommended Mitigation Steps

I suggest adding check msg.sender == _safeData[_safe].owner to ensure only the owner is allowed to call this function

Assessed type

Access Control

(non-critical) deployCamelotRelayer() does not check if whether the address is repeated or whether it already exists.

https://github.com/open-dollar/od-contracts/blob/v1.5.5-audit/src/contracts/factories/CamelotRelayerFactory.sol#L23

deployCamelotRelayer() does not check if whether the address is repeated or whether it already exists.It will cause waste of calling. I sugguest add check.

(non-critical) openSAFE() The function allows anyone to call it, which may lead to the proliferation of vault721

I suggest adding permission checks to only allow specific controller calls to prevent vault721 flooding

  /// @inheritdoc IODSafeManager
  function openSAFE(bytes32 _cType, address _usr) external returns (uint256 _id) {
    if (_usr == address(0)) revert ZeroAddress();

    ++_safeId;
    address _safeHandler = address(new SAFEHandler(safeEngine));

    _safeData[_safeId] = SAFEData({owner: _usr, safeHandler: _safeHandler, collateralType: _cType});

    _usrSafes[_usr].add(_safeId);
    _usrSafesPerCollat[_usr][_cType].add(_safeId);

    vault721.mint(_usr, _safeId);//@audit

    emit OpenSAFE(msg.sender, _usr, _safeId);
    return _safeId;
  }