Open Dollar - 0x6d6164616e's results

Lines of code

https://github.com/open-dollar/od-contracts/blob/f4f0246bb26277249c1d5afe6201d4d9096e52e6/src/contracts/AccountingEngine.sol#L215

Vulnerability details

Title

ONE_HUNDRED_WAD can cause unintended behavior in AccountingEngine.sol

Impact

The usage of ONE_HUNDRED_WAD = 100 * WAD; can result in unintended behavior when starting auctions for surplus or transferring surplus. It can lead to auctions starting with much larger amounts than intended or starting auction even tho we do not want to.

Proof of Concept

• Auction Surplus: Suppose we do not intend to transfer surplus and want to auction 100% of the surplus (_params.surplusTransferPercentage = 0). In this case:

     _amountToSell: _params.surplusAmount.wmul(ONE_HUNDRED_WAD - _params.surplusTransferPercentage),
     _initialBid: 0
   });

  • If params.surplusAmount is 100e18 (100 tokens), the calculation would be: _amountToSell = 100e18 * (100e18 - 0) / 1e18 _amountToSell = 10000e18

  This means the auction will start with an amount that is 100 times the intended surplus.

• Transfer Surplus: Suppose you do not intend to auction surplus and want to transfer 100% of the surplus amount (_params.surplusTransferPercentage = 1e18).

  let params.surplusAmount = 100e18

  _amountToSell: = 100e18*(100e18 - 1e18) / 1e18

  _amountToSell: 9900e18

  • In this case, even though you do not want to auction the surplus, an auction starts with amount 9900e18.

• Add this function( in AccountingEngine.t.sol) to test the 100 times surplusAmount

function test_surplus_transfer_Wad() public {
        uint256 surplusAmount= 100 ether;
        accountingEngine.modifyParameters("surplusTransferPercentage",abi.encode(0));
        accountingEngine.modifyParameters("extraSurplusReceiver",abi.encode(1));
        accountingEngine.modifyParameters("surplusAmount",abi.encode(surplusAmount));
        safeEngine.createUnbackedDebt(address(0),address(accountingEngine),rad(200 ether));
        uint256 id = accountingEngine.auctionSurplus();
        assertEq(surplusAuctionHouseOne.auctions(id).amountToSell, surplusAmount*100);

    }

Tools Used

vscode

Recommended Mitigation Steps

DO NOT USE ONE_HUNDRED_WAD

    if (_params.surplusTransferPercentage < WAD) {
      _id = surplusAuctionHouse.startAuction({
        _amountToSell: _params.surplusAmount.wmul(WAD - _params.surplusTransferPercentage),
        _initialBid: 0
      });

      lastSurplusTime = block.timestamp;
      emit AuctionSurplus(_id, 0, _params.surplusAmount.wmul(WAD - _params.surplusTransferPercentage));
    }

    // transfer surplus percentage
    if (_params.surplusTransferPercentage > 0) {
      if (extraSurplusReceiver == address(0)) revert AccEng_NullSurplusReceiver();

      safeEngine.transferInternalCoins({
        _source: address(this),
        _destination: extraSurplusReceiver,
        _rad: _params.surplusAmount.wmul(_params.surplusTransferPercentage)
      });

      lastSurplusTime = block.timestamp;
      emit TransferSurplus(extraSurplusReceiver, _params.surplusAmount.wmul(_params.surplusTransferPercentage));
    }```








## Assessed type

Other

Lines of code

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

Vulnerability details

Impact

In the CamelotRelayer.sol contract, there is a line of code that sets the multiplier variable as 18 - IERC20Metadata(_quoteToken).decimals();. This code snippet implies that the contract expects the decimals of the quoteToken to be 18. However, if the quoteToken has more than 18 decimal places, the subtraction 18 - IERC20Metadata(_quoteToken).decimals() may result in an underflow issue, leading to a contract revert. This behavior can prevent the contract from deploying as intended.

Proof of Concept

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

• If the quoteToken has more than 18 decimal places, the subtraction could result in an underflow and cause the contract to revert.

Tools Used

Visual Studio Code

Recommended Mitigation Steps

To address this issue and make the CamelotRelayer.sol contract more versatile, consider modifying the code to handle tokens with more than 18 decimal places without causing an underflow issue. This modification will make the contract compatible with a wider range of tokens and prevent unintended behavior.

Assessed type

Other

Lines of code

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

Vulnerability details

Impact

The CamelotRelayer.sol contract uses UniswapV3's OracleLibrary, which is designed for pairs deployed by UniswapV3's factory. However, the functions defined in this library cannot be used for Camelot pairs deployed by the CamelotV3 factory. As a result, none of the functions within CamelotRelayer.sol will work as intended when attempting to retrieve price data from Camelot pairs.

Proof of Concept

• Code reference:
  • CamelotRelayer.sol - Line 70
  • In the function getResultWithValidity(), one of the functions from UniswapV3's OracleLibrary is used, specifically, OracleLibrary.getOldestObservationSecondsAgo(camelotPair).
  • Upon inspecting the library code, it becomes clear that these functions are intended for pairs deployed by UniswapV3, and they do not work as expected with Camelot pairs.
• This is the pair deployed by camelotV3 factory
  • Example Camelot Pair
  • After reading the code of this Camelot pair, it is evident that the functions used by the OracleLibrary are not present in pairs deployed by CamelotV3.

Tools Used

vscode

Recommended Mitigation Steps

Review and update the price feed mechanism to work with Camelot pairs deployed by the CamelotV3 factory.

Assessed type

Other