Tigris Trade contest - Rolezn's results

Judge has assessed an item in Issue #164 as M risk. The relevant finding follows:

[LOW‑1] The Contract Should approve(0) First Some tokens (like USDT L199) do not work when changing the allowance from an existing non-zero allowance value. They must first be approved by zero and then the actual allowance must be approved.

Proof Of Concept 117: IERC20(assets[i]).approve(address(bondNFT), type(uint256).max); https://github.com/code-423n4/2022-12-tigris/tree/main/contracts/Lock.sol#L117

652: IERC20(_marginAsset).approve(_stableVault, type(uint).max); https://github.com/code-423n4/2022-12-tigris/tree/main/contracts/Trading.sol#L652

Recommended Mitigation Steps Approve with a zero amount first before setting the actual amount.

Lines of code

https://github.com/code-423n4/2022-12-tigris/tree/main/contracts/utils/TradingLibrary.sol#L113

Vulnerability details

Description

According to Chainlink’s documentation, the latestAnswer function is deprecated. This function might suddenly stop working if Chainlink stop supporting deprecated APIs. And the old API can return stale data.

<ins>Proof Of Concept</ins>

113: int256 assetChainlinkPriceInt = IPrice(_chainlinkFeed).latestAnswer();

https://github.com/code-423n4/2022-12-tigris/tree/main/contracts/utils/TradingLibrary.sol#L113

<ins>Recommended Mitigation Steps</ins>

Use the latestRoundData function to get the price instead. Add checks on the return data with proper revert messages if the price is stale or the round is uncomplet. See docs for reference: https://docs.chain.link/docs/price-feeds-api-reference/