Open Dollar - bitsurfer's results

Lines of code

https://github.com/open-dollar/od-contracts/blob/v1.5.5-audit/src/contracts/AccountingEngine.sol#L199

Vulnerability details

Impact

Wrong surplusTransferPercentage check condition in auctionSurplus. Instead of under 100 WAD, it's currently check if the percentage is under 1 WAD, resulting major issue of not accepting correct surplus transfer percentage in a range of 1 - 100 WAD

Proof of Concept

There is a clear wrong check in AccountingEngine's auctionSurplus function where it should be checking surplusTransferPercentage > ONE_HUNDRED_WAD instead of surplusTransferPercentage > WAD

File: AccountingEngine.sol
199:     if(_params.surplusTransferPercentage > WAD) revert AccEng_surplusTransferPercentOverLimit();

With current code, the surplusTransferPercentage will only accept if the number is under a WAD. Meanwhile there is also check _params.surplusTransferPercentage < ONE_HUNDRED_WAD in Line 213, which is contrary with the line 199, as ONE_HUNDRED_WAD is 100 * WAD.

Tools Used

Manual Analysis

Recommended Mitigation Steps

Replace WAD to ONE_HUNDRED_WAD,

if(_params.surplusTransferPercentage > ONE_HUNDRED_WAD) revert AccEng_surplusTransferPercentOverLimit();

Assessed type

Invalid Validation

Lines of code

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

Vulnerability details

Impact

Wrong addresses for UniV3Relayer and CamelotRelayer (using GOERLI)

Proof of Concept

In UniV3Relayer's _UNI_V3_FACTORY it still use GOERLI instead of UNISWAP_V3_FACTORY. Same case with CamelotRelayer.

In the UniV3Relayer.sol file, on line 18, the constant _UNI_V3_FACTORY is set to GOERLI_UNISWAP_V3_FACTORY. This should likely be replaced with the correct address, which is UNISWAP_V3_FACTORY.

Similarly, in the CamelotRelayer.sol file, on line 20, the constant _CAMELOT_FACTORY is set to GOERLI_CAMELOT_V3_FACTORY. This should be updated to the correct address, which is CAMELOT_V3_FACTORY.

File: UniV3Relayer.sol
16: contract UniV3Relayer is IBaseOracle, IUniV3Relayer {
17:   // --- Registry ---
18:   address internal constant _UNI_V3_FACTORY = GOERLI_UNISWAP_V3_FACTORY;

File: CamelotRelayer.sol
18: contract CamelotRelayer is IBaseOracle, ICamelotRelayer {
19:   // --- Registry ---
20:   address internal constant _CAMELOT_FACTORY = GOERLI_CAMELOT_V3_FACTORY;

Understandably this might be because developer forgot to replace test variable to the intended one, but still this is a medium issue, wrong 'hardcoded' address may resulting a brick of the protocol.

Tools Used

Manual Analysis

Recommended Mitigation Steps

Use the correct address of Uniswap and camelot v3 factory

Assessed type

Other