code-423n4 / 2022-12-tigris-findings

8 stars 4 forks source link

TradingLibrary> verifyPrice may still verifies the wrong price as valid #44

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

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

GalloDaSballo commented 1 year ago

Missing further detail but ultimately is dup of more developed reports, will award 25%

c4-judge commented 1 year ago

GalloDaSballo marked the issue as duplicate of #108

c4-judge commented 1 year ago

GalloDaSballo marked the issue as partial-25

c4-judge commented 1 year ago

GalloDaSballo changed the severity to 2 (Med Risk)