Open Dollar - twcctop's results

Lines of code

https://github.com/open-dollar/od-contracts/blob/b181f77b16bb874dc23eef50f67b21c08ec15481/src/contracts/oracles/CamelotRelayer.sol#L58 https://github.com/open-dollar/od-contracts/blob/c328d560e1540a8079fa30ed98c355bf10f498ed/src/contracts/oracles/UniV3Relayer.sol#L64

Vulnerability details

Impact

Relayer don't support high decimal token pair，contract creation will revert.

Proof of Concept

In relayer constructor,

  constructor(address _baseToken, address _quoteToken, uint32 _quotePeriod) {
...
    baseAmount = uint128(10 ** IERC20Metadata(_baseToken).decimals());
@>    multiplier = 18 - IERC20Metadata(_quoteToken).decimals();
    quotePeriod = _quotePeriod

For example, https://etherscan.io/token/0xAba8cAc6866B83Ae4eec97DD07ED254282f6aD8A YAMv2 has 24 decimals, 18 - IERC20Metadata(_quoteToken).decimals() will revert().

Tools Used

manual

Recommended Mitigation Steps

consider add high decimal token support.

Assessed type

Decimal

Lines of code

https://github.com/open-dollar/od-contracts/blob/a6fc5524404acb56f9a69d93661fe99cf2bea877/src/contracts/proxies/Vault721.sol#L140

Vulnerability details

Impact

tokenURI don't check _safeId exist or not ,render will return error token uri msg.

Proof of Concept

 function tokenURI(uint256 _safeId) public view override returns (string memory uri) {
    uri = nftRenderer.render(_safeId);
  }

safeId is not checked exist in this function . If a non-exist _safeId is passed,the render logic the concat some 0 value token uri msg, which makes return value inaccurate.

Tools Used

manual

Recommended Mitigation Steps

check _safeId exist

Assessed type

Access Control

Lines of code

https://github.com/open-dollar/od-contracts/blob/7dbe64b53caf4f7fb122fa94b2bde502c6dea8da/src/contracts/proxies/ODSafeManager.sol#L105-L109

Vulnerability details

Impact

Safe allowed users can allow others.

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);
  }

Users can be allowed to safe . Take a look at modifier safeAllowed

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

The allowSAFE modifier is allowSAFE ,according to this , any safe allowed user could allow others.

safeCan[_owner][_safe][msg.sender] == 0 will pass if caller is safe allowed, wheather it's safe owner or not

Tools Used

Recommended Mitigation Steps

consider add only safeowner logic in allowSAFE function

Assessed type

Access Control