Open Dollar - Arz's results

Lines of code

https://github.com/open-dollar/od-contracts/blob/f4f0246bb26277249c1d5afe6201d4d9096e52e6/src/contracts/proxies/ODSafeManager.sol#L20

Vulnerability details

If a user wanted to call a function in ODSafeManager then he has to call BasicActions because its a delegatecall from the proxy and if the user called ODSafeManager directly then we are using the proxy's storage. The problem is that some functions like allowSAFE(), allowHandler(), moveSAFE() and etc do not exist in the BasicActions.sol and cant be called from there so the users wont be able to interact with these functions.

Impact

Users will be unable to call some important functions in the ODSafeManager.

Proof of Concept

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

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

If a user wanted to call allowSAFE() then he would delegatecall this contract from the proxy but when we are delegatecalling we are using the state and storage of the proxy contract so we wont modify the storage of the ODSafeManager and this wont set anything. And because this doesnt exist in the BasicActions the user will be unable to call this function and other functions.

Tools Used

Manual Review

Recommended Mitigation Steps

Add all of these functions in BasicActions.sol - allowSAFE, allowHandler, moveSAFE, addSAFE, removeSAFE, protectSAFE, quitSystem, enterSystem

Assessed type

call/delegatecall

Lines of code

https://github.com/open-dollar/od-contracts/blob/f4f0246bb26277249c1d5afe6201d4d9096e52e6/src/contracts/gov/ODGovernor.sol#L41

Vulnerability details

When ODGovernor is deployed the GovernorSettings is initialized with the initialVotingDelay, initialVotingPeriod and initialProposalThreshold. The problem is that the initialVotingDelay and initialVotingPeriod are set to incorrect values which are really small and the proposal wont have time to receive votes.

Impact

The votingPeriod and the snapshot will be too short and there will be not time to vote

Proof of Concept

https://github.com/open-dollar/od-contracts/blob/f4f0246bb26277249c1d5afe6201d4d9096e52e6/src/contracts/gov/ODGovernor.sol#L41

41:  GovernorSettings(1, 15, 0)

As you can see it is set to 1 and 15 which means that the initialVotingDelay will be 1 block and the initialVotingPeriod will be 15 blocks so there will be no time to vote.

Tools Used

Manual Review

Recommended Mitigation Steps

Use the correct values

Assessed type

Governance

Lines of code

https://github.com/open-dollar/od-contracts/blob/f4f0246bb26277249c1d5afe6201d4d9096e52e6/src/contracts/oracles/CamelotRelayer.sol#L20 https://github.com/open-dollar/od-contracts/blob/f4f0246bb26277249c1d5afe6201d4d9096e52e6/src/contracts/oracles/UniV3Relayer.sol#L18

Vulnerability details

UniV3Relayer and CamelotRelayer both have a constant of the dexs factory address which is used to get the pool addresses. The problem is that this constant is set to the goerli factory address and not the mainnet one.

Impact

When the relayers are deployed both wont work because the real mainnet address is different from the goerli one that was set and the contracts will have to be redeployed.

Proof of Concept

https://github.com/open-dollar/od-contracts/blob/f4f0246bb26277249c1d5afe6201d4d9096e52e6/src/contracts/oracles/CamelotRelayer.sol#L20 https://github.com/open-dollar/od-contracts/blob/f4f0246bb26277249c1d5afe6201d4d9096e52e6/src/contracts/oracles/UniV3Relayer.sol#L18

18:  address internal constant _UNI_V3_FACTORY = GOERLI_UNISWAP_V3_FACTORY;


20: address internal constant _CAMELOT_FACTORY = GOERLI_CAMELOT_V3_FACTORY;

As you can see both constants are set to the goerli address instead of the mainnet one.

Tools Used

Manual Review

Recommended Mitigation Steps

Set it to the mainnet address

address internal constant _UNI_V3_FACTORY = UNISWAP_V3_FACTORY;


address internal constant _CAMELOT_FACTORY = CAMELOT_V3_FACTORY;

Assessed type

Other

Lines of code

https://github.com/open-dollar/od-contracts/blob/f4f0246bb26277249c1d5afe6201d4d9096e52e6/src/contracts/oracles/CamelotRelayer.sol#L10 https://github.com/Uniswap/v3-periphery/blob/697c2474757ea89fec12a4e6db16a574fe259610/contracts/libraries/OracleLibrary.sol#L28

Vulnerability details

The CamelotRelayer is using the Uniswap OracleLibrary but the problem is that the function names of some functions of Camelot differ from the Uniswap function names so the library is not going to work.

When calling OracleLibrary.consult() in the library it will then call IUniswapV3Pool(pool).observe which can be found in the UniswapV3PoolDerivedState here however if you check the PoolDerivedState of Camelot here(this is the address of the wstETH Camelot pool) you can see that the function names are different so when calling observe() the tx will revert because the Camelot contracts dont have this function.

Impact

The CamelotRelayer wont work and the tx will always revert when getResultWithValidity() is called

Proof of Concept

https://github.com/Uniswap/v3-periphery/blob/697c2474757ea89fec12a4e6db16a574fe259610/contracts/libraries/OracleLibrary.sol#L28

28: IUniswapV3Pool(pool).observe(secondsAgos);

As you can see observe() is called when consult() is called in the library but if you check the Camelot pool you could see that observe() does not exist and its called getTimepoints() instead.

Tools Used

Manual Review

Recommended Mitigation Steps

Create a library for Camelot and use the correct function names.

Assessed type

Library