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

8 stars 4 forks source link

Upgraded Q -> M from #164 [1674419095024] #670

Closed c4-judge closed 1 year ago

c4-judge commented 1 year ago

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.

c4-judge commented 1 year ago

GalloDaSballo marked the issue as duplicate of #198

c4-judge commented 1 year ago

GalloDaSballo marked the issue as satisfactory