Tigris Trade contest - Critical's results

Lines of code

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

Vulnerability details

Impact

When any of the listed stablecoins depeg, the bad coins will drive out the good coins, leaving only bad coins in StableVault, resulting in the collapse of StableVault.

Proof of Concept

Given:

StableVault listed three stablecoins: USDD, USDC, and USDT.

1. Alice deposited 1,000 USDC and got 1,000 tigUSD (stable) back.
2. USDD depegged to 0.8 USD.
3. Bob bought 1000 USDD with 800 USDC, then deposited to StableVault, getting 1,000 tigUSD.
4. Bob called a withdrawal burn of 1,000 tigUSD and got 1,000 USDC, earning 200 USDC.

Alice could not get any USDC back; she could only choose to withdraw USDD and bear the loss.

Recommended Mitigation Steps

Each StableVault should only be allowed to wrap one token to its corresponding tigAsset.

Lines of code

https://github.com/code-423n4/2022-12-tigris/blob/588c84b7bb354d20cbca6034544c4faa46e6a80e/contracts/utils/TradingLibrary.sol#L91-L122

Vulnerability details

Impact

initiateMarketOrder() can be called with a signed off-chain price data from the past _validSignatureTimer and that price will be taken as the open price of the position.

Because the difference of the current price to the price from _validSignatureTimer ago is known to the caller, it will be possible for the user to create a long position at a price 2% lower than the current market price, or a short position at a price 2% higher than the current market price.

If the _validSignatureTimer is long enough, combining with leverage, the attacker will be able to disrupt the market by draining all the profits from the other regular users.

https://github.com/code-423n4/2022-12-tigris/blob/588c84b7bb354d20cbca6034544c4faa46e6a80e/contracts/utils/TradingLibrary.sol#L91-L122

function verifyPrice(
    uint256 _validSignatureTimer,
    uint256 _asset,
    bool _chainlinkEnabled,
    address _chainlinkFeed,
    PriceData calldata _priceData,
    bytes calldata _signature,
    mapping(address => bool) storage _isNode
)
    external view
{
    address _provider = (
        keccak256(abi.encode(_priceData))
    ).toEthSignedMessageHash().recover(_signature);
    require(_provider == _priceData.provider, "BadSig");
    require(_isNode[_provider], "!Node");
    require(_asset == _priceData.asset, "!Asset");
    require(!_priceData.isClosed, "Closed");
    require(block.timestamp >= _priceData.timestamp, "FutSig");
    require(block.timestamp <= _priceData.timestamp + _validSignatureTimer, "ExpSig");
    require(_priceData.price > 0, "NoPrice");
    if (_chainlinkEnabled && _chainlinkFeed != address(0)) {
        int256 assetChainlinkPriceInt = IPrice(_chainlinkFeed).latestAnswer();
        if (assetChainlinkPriceInt != 0) {
            uint256 assetChainlinkPrice = uint256(assetChainlinkPriceInt) * 10**(18 - IPrice(_chainlinkFeed).decimals());
            require(
                _priceData.price < assetChainlinkPrice+assetChainlinkPrice*2/100 &&
                _priceData.price > assetChainlinkPrice-assetChainlinkPrice*2/100, "!chainlinkPrice"
            );
        }
    }
}

Recommended Mitigation Steps

Signed _priceData should only be allowed for SL/TP and match limit orders. For those orders, only a priceData with a timestamp after the created timestamp of the order can be used.

Lines of code

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

Vulnerability details

Impact

function deposit(address _token, uint256 _amount) public {
        require(allowed[_token], "Token not listed");
        IERC20(_token).transferFrom(_msgSender(), address(this), _amount);
        IERC20Mintable(stable).mintFor(
            _msgSender(),
            _amount*(10**(18-IERC20Mintable(_token).decimals()))
        );
    }

deposit will revert at L49 for tokens with decimals > 18.

Recommended Mitigation Steps

Consider checking if the token's decimals < 18 in listToken().