Tigris Trade contest - 8olidity's results

Lines of code

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

Vulnerability details

Impact

In deposit(), the user can send a certain amount of token and then get a certain amount of stable token. The original design here should only support stable coins for token. But there is no operation to judge the price of stablecoins here. Too much trust in the price of stablecoins. We should learn from the events of USTC. Once the price of a certain stablecoin falls. The stablevault contract is not aware of price changes. Then the attacker can use the low-priced token to deposit(), exchange for stable token, and then redeem other stable coins. Profit from it.

    function deposit(address _token, uint256 _amount) public {// @audit 
        require(allowed[_token], "Token not listed");
        IERC20(_token).transferFrom(_msgSender(), address(this), _amount);
        IERC20Mintable(stable).mintFor(
            _msgSender(),
            _amount*(10**(18-IERC20Mintable(_token).decimals())) // 不同的decimals代币，计算之后都是一样的值。
        );
    }

Proof of Concept

You can also refer to arbitrageurs in ankr, because there is no immediate price update in the Helio protocol. cause to be attacked https://twitter.com/PeckShieldAlert/status/1598527823224111104

Tools Used

vscode

Recommended Mitigation Steps

A price oracle should be added to the deposit

Lines of code

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

Vulnerability details

Impact

When using the approval mechanism in USDT, the approval must be set to 0 before it is updated. https://etherscan.io/address/0xdac17f958d2ee523a2206206994597c13d831ec7#code

require(!((_value != 0) && (allowed[msg.sender][_spender] != 0)));

When _tigAsset is USDT, _handleCloseFees() should first approve(0) and then re-approve, in order to solve the problem that the user has operated before

Proof of Concept

        IStable(_tigAsset).mintFor(address(this), _daoFeesPaid);
        IStable(_tigAsset).approve(address(gov), type(uint).max);
        gov.distribute(_tigAsset, _daoFeesPaid);

Tools Used

vscode

Recommended Mitigation Steps

Set the allowance to 0 before setting it to the new value.

Lines of code

https://github.com/code-423n4/2022-12-tigris/blob/588c84b7bb354d20cbca6034544c4faa46e6a80e/contracts/GovNFT.sol#L101 https://github.com/code-423n4/2022-12-tigris/blob/588c84b7bb354d20cbca6034544c4faa46e6a80e/contracts/BondNFT.sol#L336 https://github.com/code-423n4/2022-12-tigris/blob/588c84b7bb354d20cbca6034544c4faa46e6a80e/contracts/GovNFTBridged.sol#L79

Vulnerability details

Impact

The GovNFT/BondNFT/GovNFTBridged contract allows users to call safeTransferMany() to send nft to the to address, and safeTransferMany() bottom layer will call _transfer() to send nft, but here if the to address does not support ERC721. Then the nft will be locked in the to address.

Proof of Concept

    function _transfer(
        address from,
        address to,
        uint256 tokenId
    ) internal override {
        require(ownerOf(tokenId) == from, "!Owner");
        for (uint i=0; i<assetsLength(); i++) {
            userDebt[from][assets[i]] += accRewardsPerNFT[assets[i]];
            userDebt[from][assets[i]] -= userPaid[from][assets[i]]/balanceOf(from);
            userPaid[from][assets[i]] -= userPaid[from][assets[i]]/balanceOf(from);
            userPaid[to][assets[i]] += accRewardsPerNFT[assets[i]];
        }
        super._transfer(from, to, tokenId);//@audit
    }

Tools Used

vscode

Recommended Mitigation Steps

It is recommended to use safeTransferFrom() to send nft

Lines of code

https://github.com/code-423n4/2022-12-tigris/blob/588c84b7bb354d20cbca6034544c4faa46e6a80e/contracts/PairsContract.sol#L104-L108

Vulnerability details

Impact

Administrators can set feeMultiplier to any value through updateAssetFeeMultiplier(). If set too large. The user's funds will all pay the handling fee, for example, in _handleCloseFees(), if the sum of handling fees is equal to _payout. Then the final _payout will be 0, and all funds are used to pay the handling fee

        payout_ = _payout - _daoFeesPaid - _burnFeesPaid - _botFeesPaid;
        IStable(_tigAsset).mintFor(address(this), _daoFeesPaid);
        IStable(_tigAsset).approve(address(gov), type(uint).max);
        gov.distribute(_tigAsset, _daoFeesPaid);
        return payout_;

Proof of Concept

    function updateAssetFeeMultiplier(uint256 _asset, uint256 _feeMultiplier) external onlyOwner {
        bytes memory _name  = bytes(_idToAsset[_asset].name);
        require(_name.length > 0, "!Asset");
        _idToAsset[_asset].feeMultiplier = _feeMultiplier;
    }

Tools Used

vscode

Recommended Mitigation Steps

Set the maximum value of feeMultiplier

Lines of code

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

Vulnerability details

Impact

latestAnswer() this method will return the last value, but you won't be able to check if the data is fresh. On the other hand, calling the method latestRoundData allow you to run some extra validations

ref:https://github.com/code-423n4/2021-07-wildcredit-findings/issues/75

Proof of Concept

    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();

Tools Used

vscode

Recommended Mitigation Steps

Use V3 interface functions: https://docs.chain.link/docs/price-feeds-api-reference/