code-423n4 / 2022-06-connext-findings

1 stars 0 forks source link

`getPriceFromDex()` in `ConnextPriceOracle.sol` will always fail for tokens with more than 18 decimals due to airthmetic underflow #191

Closed code423n4 closed 2 years ago

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#L103 https://github.com/code-423n4/2022-06-connext/blob/b4532655071566b33c41eac46e75be29b4a381ed/contracts/contracts/core/connext/helpers/ConnextPriceOracle.sol#L106

Vulnerability details

Impact

Calls to getPriceFromDex() for a specific tokenAddress will always fail if that token has more than 18 decimals. Thus, calls to getTokenPrice() which call getPriceFromDex() as a backup for the Chainlink oracle feed return data will also fail, and thus the platform will not be able to report a price for that token.

While this prevents all usage of that token on Connext platform, since the functions are only executed for registered tokens, I think a severity of Medium is warranted as it is plausible that admins will vet incoming tokens.

Proof of Concept

ConnextPriceOracle.sol:L103

uint256 tokenDecimalDelta = 18 - uint256(IERC20Extended(priceInfo.token).decimals())

Since the version of solidity used in this file is pragma solidity 0.8.14; , arithmetic overflow/underflow is automatically checked and an error is thrown.

Tools Used

Manual review

Recommended Mitigation Steps

Check that the .decimals() of the token is <= 18, and adjust the operation accordinly. Or, if tokens with more than 18 decimals are not anticipated to be allowed, restrict their addition to priceRecords.

ecmendenhall commented 2 years ago

Duplicate of #39

jakekidd commented 2 years ago

dup #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.

0xleastwood commented 2 years ago

Merging with #190.