Tigris Trade contest - rotcivegaf's results

Lines of code

https://github.com/code-423n4/2022-12-tigris/blob/588c84b7bb354d20cbca6034544c4faa46e6a80e/contracts/Position.sol#L126-L161 https://github.com/code-423n4/2022-12-tigris/blob/588c84b7bb354d20cbca6034544c4faa46e6a80e/contracts/Trading.sol#L156-L210 https://github.com/code-423n4/2022-12-tigris/blob/588c84b7bb354d20cbca6034544c4faa46e6a80e/contracts/Trading.sol#L307-L349

Vulnerability details

Author: rotcivegaf

Impact

The root of the problem are in the mint who use _safeMint. This mint call the function onERC721Received of the token recipient, generating a possible reentrancy attack

Proof of Concept

A wallet can call the initiateMarketOrder or initiateLimitOrder and this functions call the mint function who inside call the OZ _safeMint When the the _safeMint call onERC721Received of the _mintTrade.account, this one have the possibility to call functions and the lines L149-L160 of mint are not execute yet, also L207-L210 of initiateMarketOrder and L347-L348 of initiateLimitOrder The _mintTrade.account can call:

• executeLimitOrder and avoid require(block.timestamp >= limitDelay[_id]);
• cancelLimitOrder, liquidatePosition, initiateCloseOrder and limitClose burning the nft

Tools Used

Review

Recommended Mitigation Steps

Can use reentrancy guards in the three function mint, initiateMarketOrder and initiateLimitOrder Or use _mint instead of _safeMint

Lines of code

https://github.com/code-423n4/2022-12-tigris/blob/588c84b7bb354d20cbca6034544c4faa46e6a80e/contracts/StableVault.sol#L44-L72 https://github.com/code-423n4/2022-12-tigris/blob/588c84b7bb354d20cbca6034544c4faa46e6a80e/contracts/StableVault.sol#L74-L96

Vulnerability details

Author: rotcivegaf

Impact

The stable coins to USD don't follow the USD exactly, there are variations between this tokens and this give the possibility of arbitrate the stable tokens listed on the StableVault, for example Tether goes down to price 0.9485/USD in this year This gets worse if some stable token crash/hack, like Iron Finance’s Titan Token or LUNA of terra, the users can deposit the crashed stable token and take the not crashed stable tokens even if the crashed stable token is deslisted Another possibility to broke this contract it's list a not stable token or list the stable token of the StableVault

Proof of Concept

1. For example if DAI token goes down, deposit DAI token, withdraw another stable token and trade with DAI token, getting more DAI. This is a simple arbitrage example

2. The real damage it's if one of the stable coin crash:

• We have 2 tokens: LUNA and DAI
• The stable token LUNA goes down
• Alice deposit the balance of StableVault contract in LUNA tokens to withdraw the same value of DAI tokens

Finally Alice get all DAI tokens of the StableVault contract and the contract have only LUNA tokens, the users who deposited DAI can't withdraw DAI only can withdraw LUNA tokens who his value it's 0.8 and probably go to 0

Note: In argentina we have a similar case to this: Corralito

3. If the owner list the stable token: Alice deposit this stable token to withdraw all the balance of the others stable tokens, leaving the StableVault contract alone with stable token. The others users will only withdraw the stable token

Recommended Mitigation Steps

The function listToken should revert if the _token is the stable token

The StableVault have severals critical points, one possible mitigation it's limit the total supply of the stable mint to low the the risk