Denial of service with tokens with > 18 decimals

[code423n4]

Lines of code

https://github.com/code-423n4/2022-06-connext/blob/07adce8e88d3c93e26d32c9c2056593c62911197/contracts/contracts/core/connext/helpers/ConnextPriceOracle.sol#L103
https://github.com/code-423n4/2022-06-connext/blob/07adce8e88d3c93e26d32c9c2056593c62911197/contracts/contracts/core/connext/helpers/ConnextPriceOracle.sol#L134
https://github.com/code-423n4/2022-06-connext/blob/07adce8e88d3c93e26d32c9c2056593c62911197/contracts/contracts/core/connext/facets/StableSwapFacet.sol#L426

Vulnerability details

Impact

Some contracts assumes decimals <= 18 and does not handle > 18 decimals.

Proof of Concept

Because the pragma used doesn't allow integer underflows, if a token with more than 18 decimals is used, an integer underflow will produce a denial of service.

Some tokens have more than 18 decimals (e.g. YAM-V2 has 24).

This may trigger unexpected reverts due to overflow, posing a liveness risk to the contract.

Reference:

Major severity finding from Consensys Diligence Audit of Defi Saver:

• https://consensys.net/diligence/audits/2021/03/defi-saver/#tokens-with-more-than-18-decimal-points-will-cause-issues

Affected source code:

• https://github.com/code-423n4/2022-06-connext/blob/07adce8e88d3c93e26d32c9c2056593c62911197/contracts/contracts/core/connext/helpers/ConnextPriceOracle.sol#L103
• https://github.com/code-423n4/2022-06-connext/blob/07adce8e88d3c93e26d32c9c2056593c62911197/contracts/contracts/core/connext/helpers/ConnextPriceOracle.sol#L134
• https://github.com/code-423n4/2022-06-connext/blob/07adce8e88d3c93e26d32c9c2056593c62911197/contracts/contracts/core/connext/facets/StableSwapFacet.sol#L426

Recommended Mitigation Steps

Ascertain that the code will not fail if the token's decimals are greater than 18.

[ecmendenhall]

Duplicate of #39.

[ecmendenhall]

I gave this a ❤️ along with #204 because these findings both identified an additional location in the StableSwap contract where the 18 decimal assumption is hardcoded.

[jakekidd]

Marking this as duplicate but leaving it open/confirmed since it identifies that additional location where the assumption is hardcoded!

[jakekidd]

Assortment of findings across these three issues:
#39
#61
#204

[jakekidd]

Fixed by connext/monorepo@f2e5b66

[0xleastwood]

Duplicate of #204.