code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2022-12-tigris/blob/main/contracts/Trading.sol#L650 https://github.com/code-423n4/2022-12-tigris/blob/main/contracts/Trading.sol#L675

Vulnerability details

Impact

In Trading.sol a deposit or withdrawal of tokens with decimals higher than 18 will always revert.

This is the case e.g. for NEAR which is divisible into 10e24 yocto

Proof of Concept

Change 00.Mocks.js#L33 to:

args: ["USDC", "USDC", 24, deployer, ethers.utils.parseUnits("1000", 24)]

Then in 07.Trading.js:

Opening and closing a position with tigUSD output
Opening and closing a position with <18 decimal token output

are going to fail with:

Error: VM Exception while processing transaction: reverted with panic code 0x11 (Arithmetic operation underflowed or overflowed outside of an unchecked block)

Tools Used

Visual Studio Code

Recommended Mitigation Steps

Update calculations in the contract to account for tokens with decimals higher than 18.

GalloDaSballo commented 1 year ago

Best because it has a simple POC and is concise

c4-judge commented 1 year ago

GalloDaSballo marked the issue as primary issue

TriHaz commented 1 year ago

We are aware of that, we are not planning on adding any token that has more than 18 dec.

c4-sponsor commented 1 year ago

TriHaz marked the issue as sponsor acknowledged

GalloDaSballo commented 1 year ago

The Warden has shown how, due to an underflow, the system in-scope can revert when using tokens with more than 18 decimals.

Because of how scope was defined, I believe the finding to be valid, I believe a nofix is acceptable as long as the sponsor keeps in mind this risk.

Because of the risk shown, I agree with Medium Severity