search

code-423n4 / 2023-09-centrifuge-findings

16 stars 14 forks source link

Formatting Messages Excluding Key Argument #634

Closed c4-submissions closed 1 year ago

c4-submissions commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-09-centrifuge/blob/512e7a71ebd9ae76384f837204216f26380c9f91/src/gateway/Gateway.sol#L208

Vulnerability details

Bug Description

When an investor invokes the LiquidityPool.requestDeposit function, a message with important arguments is sent to the CentrifugeGateway smart contract through Axelar's General Message Passing mechanism. The increaseInvestOrder message consists of the following arguments: poolId, trancheId, investor, currencyId, and currencyAmount. An important argument that is not passed in this case is the decimals of currency.

Proof-of-Concept

For example, the smart contracts will be deployed on both Ethereum and BSC (Binance Smart Chain). Some ERC20 tokens have different decimals on different chains. Even some popular ones like USDT and USDC have 6 decimals on Ethereum, and 18 decimals on BSC for example:

USDT on Ethereum - 6 decimals USDC on Ethereum - 6 decimals USDT on BSC - 18 decimals USDC on BSC - 18 decimals

Let's examine the next two examples: Messages.formatIncreaseInvestOrder(poolId, trancheId, _addressToBytes32(investor), currencyId, currencyAmount)

  1. Bob sends the following formatted message from LiquidityPool on Ethereum to the CFG chain (currencyAmount = 1e6).
  2. Bob sends the following formatted message from LiquidityPool on BSC to the CFG chain (currencyAmount = 1e18).

CentrifugeGateway may not anticipate that currencies with the same ID may have different decimals.

Impact

Due to the fact that currencies with the same ID but different decimals can be sent to the centrifugeGatewayPrecompileAddress (0x0000000000000000000000000000000000002048), errors in the calculation of the amount of Tranche tokens may occur.

Tools Used

Manual

Recommended Mitigation Steps

To prevent such situations from occurring, it is advisable to add an additional argument, currencyDecimals, to all places where messages contain currencyId and currencyAmount. This approach will ensure that the correct decimals are considered in calculations, thereby helping to avoid errors in Tranche token amount calculations.

Assessed type

Decimal

c4-pre-sort commented 1 year ago

raymondfam marked the issue as low quality report

c4-pre-sort commented 1 year ago

raymondfam marked the issue as duplicate of #78

c4-pre-sort commented 1 year ago

raymondfam marked the issue as not a duplicate

c4-pre-sort commented 1 year ago

raymondfam marked the issue as primary issue

c4-pre-sort commented 1 year ago

raymondfam marked the issue as sufficient quality report

c4-sponsor commented 1 year ago

hieronx (sponsor) disputed

hieronx commented 1 year ago

Currency IDs are globally unique: USDC on Ethereum will have a different currency ID than USDC on BSC. Centrifuge Chain bookkeeps what decimals each currency ID has, and accounts for this accordingly.

c4-judge commented 1 year ago

gzeon-c4 marked the issue as unsatisfactory: Invalid