code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-06-connext/blob/b4532655071566b33c41eac46e75be29b4a381ed/contracts/contracts/core/connext/helpers/ConnextPriceOracle.sol#L99-L115

Vulnerability details

Impact

the value 18 is hardcoded in the code and expression 18 - uint256(IERC20Extended(priceInfo.token).decimals()) is in code so if a token had more than 18 precision then this contract couldn't support it and all the logics based on this contract would fail for those tokens.

Proof of Concept

This is getPriceFromDex() code in ConnextPriceOracle:

  function getPriceFromDex(address _tokenAddress) public view returns (uint256) {
    PriceInfo storage priceInfo = priceRecords[_tokenAddress];
    if (priceInfo.active) {
      uint256 rawTokenAmount = IERC20Extended(priceInfo.token).balanceOf(priceInfo.lpToken);
      uint256 tokenDecimalDelta = 18 - uint256(IERC20Extended(priceInfo.token).decimals());
      uint256 tokenAmount = rawTokenAmount.mul(10**tokenDecimalDelta);
      uint256 rawBaseTokenAmount = IERC20Extended(priceInfo.baseToken).balanceOf(priceInfo.lpToken);
      uint256 baseTokenDecimalDelta = 18 - uint256(IERC20Extended(priceInfo.baseToken).decimals());
      uint256 baseTokenAmount = rawBaseTokenAmount.mul(10**baseTokenDecimalDelta);
      uint256 baseTokenPrice = getTokenPrice(priceInfo.baseToken);
      uint256 tokenPrice = baseTokenPrice.mul(baseTokenAmount).div(tokenAmount);

      return tokenPrice;
    } else {
      return 0;
    }
  }

As you can see the code don't support tokens with more than 18 digit precision and execution would revert for those tokens.

Tools Used

VIM

Recommended Mitigation Steps

add support for all tokens.

ecmendenhall commented 2 years ago

Duplicate of #39

jakekidd commented 2 years ago

Duplicate of https://github.com/code-423n4/2022-06-connext-findings/issues/39

jakekidd commented 2 years ago

Duplicate of https://github.com/code-423n4/2022-06-connext-findings/issues/39

0xleastwood commented 2 years ago

Completely valid issue but the affected function is not being actively used in the codebase. Downgrading to QA as it may be integrated in the future.