code-423n4 / 2023-05-maia-findings

24 stars 13 forks source link

BranchPort does not take into account tokens with varying decimals #276

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-05-maia/blob/54a45beb1428d85999da3f721f923cbf36ee3d35/src/ulysses-omnichain/RootPort.sol#L282 https://github.com/code-423n4/2023-05-maia/blob/54a45beb1428d85999da3f721f923cbf36ee3d35/src/ulysses-omnichain/BranchPort.sol#L248 https://github.com/code-423n4/2023-05-maia/blob/54a45beb1428d85999da3f721f923cbf36ee3d35/src/ulysses-omnichain/BranchPort.sol#L275

Vulnerability details

Impact

Root hTokens will be minted based on the decimals of the underlying deposit, which causes incorrect accounting.

Proof of Concept

In bridgeToRoot() of RootPort.sol, the root hToken is transferred to the _recipient. The amount that is transferred is the difference between _amount and _deposit. As discussed with the Sponsor, _amount is an amount of hTokens, which is decimal 18. The _deposit is the deposit amount of underlying, which can have varying decimals. This results in incorrect accounting, because _deposit amounts using decimals lower than 18 will result in more tokens transferred. Tokens with decimals greater than 18, can result in an underflow because _deposit is greater than _amount.

There are other places where amount - deposit is used, but does not take into account the deposit decimals, such as:

Tools Used

Manual

Recommended Mitigation Steps

Consider converting _deposit to 18 decimals to match the hToken decimals.

Assessed type

Decimal

c4-judge commented 1 year ago

trust1995 marked the issue as duplicate of #758

c4-judge commented 1 year ago

trust1995 marked the issue as satisfactory

c4-judge commented 1 year ago

trust1995 marked the issue as partial-50

c4-judge commented 1 year ago

trust1995 marked the issue as full credit

c4-judge commented 1 year ago

trust1995 marked the issue as satisfactory