Platform: Code4rena
Start Date: 09/12/2022
Pot Size: $90,500 USDC
Total HM: 35
Participants: 84
Period: 7 days
Judge: GalloDaSballo
Total Solo HM: 12
Id: 192
League: ETH
Rank: 59/84
Findings: 3
Award: $73.21
π Selected for report: 0
π Solo Findings: 0
π Selected for report: 0x4non
Also found by: 0xNazgul, Deivitto, __141345__, cccz, eierina, imare, kwhuo68, rvierdiiev
60.3691 USDC - $60.37
https://github.com/code-423n4/2022-12-tigris/blob/main/contracts/Trading.sol#L652 https://github.com/code-423n4/2022-12-tigris/blob/main/contracts/Lock.sol#L117
There are some instances where there is an ERC20 approval for a max uint256 amount. ERC20 tokens such as USDT require the address allowance to be set to 0 beforehand, so this would cause reverts for those tokens.
-Token such as USDT gets whitelisted
-Normal call for to stableVault contract will revert, via https://github.com/code-423n4/2022-12-tigris/blob/main/contracts/Trading.sol#L652 since USDT requires an approval of 0 first
-This can also apply in the Lock, asset distribution to bondNFT contract: https://github.com/code-423n4/2022-12-tigris/blob/main/contracts/Lock.sol#L117
None
In the above scenarios, precede the corresponding lines with the line: IERC20(_asset).approve(_spender) ,0) to account for that edge case.
#0 - c4-judge
2022-12-20T15:49:30Z
GalloDaSballo marked the issue as duplicate of #104
#1 - c4-judge
2023-01-22T17:45:44Z
GalloDaSballo marked the issue as satisfactory
π Selected for report: 0xA5DF
Also found by: 0xA5DF, 0xNazgul, 0xSmartContract, 0xbepresent, 0xdeadbeef0x, 8olidity, Englave, Faith, HE1M, JohnnyTime, Madalad, Mukund, Ruhum, SmartSek, __141345__, aviggiano, carlitox477, cccz, chaduke, francoHacker, gz627, gzeon, hansfriese, hihen, imare, jadezti, kwhuo68, ladboy233, orion, peanuts, philogy, rbserver, wait, yjrwkk
1.1472 USDC - $1.15
https://github.com/code-423n4/2022-12-tigris/blob/main/contracts/Trading.sol#L939 https://github.com/code-423n4/2022-12-tigris/blob/main/contracts/Trading.sol#L496
Can cause issues in executeLimitOrder() to always revert unless price = trade.price
-Set _range to 0 in setLimitOrderPriceRange() in https://github.com/code-423n4/2022-12-tigris/blob/main/contracts/Trading.sol#L939 -This then causes executeLimitOrder() to always revert unless price = trade.price via https://github.com/code-423n4/2022-12-tigris/blob/main/contracts/Trading.sol#L496
None
Add in a check so that _range cannot be 0 in setLimitOrderPriceRange()
#0 - TriHaz
2023-01-09T16:48:53Z
Ideally, setting _rage
to 0 shouldn't happen, but a check in setLimitOrderPriceRange()
should be added, I would change severity to QA.
#1 - c4-sponsor
2023-01-09T16:48:59Z
TriHaz marked the issue as sponsor confirmed
#2 - c4-sponsor
2023-01-09T16:49:05Z
TriHaz marked the issue as disagree with severity
#3 - c4-judge
2023-01-19T19:51:02Z
GalloDaSballo marked the issue as duplicate of #377
#4 - c4-judge
2023-01-22T17:28:47Z
GalloDaSballo marked the issue as satisfactory
π Selected for report: rbserver
Also found by: 0x52, 0xDecorativePineapple, 0xdeadbeef0x, 8olidity, Jeiwan, Rolezn, __141345__, bin2chen, eierina, fs0c, gzeon, joestakey, koxuan, kwhuo68, ladboy233, rvierdiiev, yixxas
11.6941 USDC - $11.69
https://github.com/code-423n4/2022-12-tigris/blob/main/contracts/utils/TradingLibrary.sol#L113
ChainLink oracle could be using stale data which can cause unexpected behavior in trading and make trading susceptible to oracle attacks (flash loans etc).
In verifyPrice() the ChainLink feed should use latestRoundData() instead of latestAnswer() https://github.com/code-423n4/2022-12-tigris/blob/main/contracts/utils/TradingLibrary.sol#L113 since latestAnswer() returns the last value but the data could be stale. Using latestRoundData() allows for checking the updateTime and answeredInRound fields for staleness validation.
None
Use (roundId, price, , updateTime, answeredInRound) = IPrice(_chainlinkFeed).latestRoundData() and add latestRoundData() to IPrice interface.
#0 - c4-judge
2022-12-20T16:34:41Z
GalloDaSballo marked the issue as duplicate of #655
#1 - c4-judge
2023-01-22T17:30:24Z
GalloDaSballo marked the issue as satisfactory