Platform: Code4rena
Start Date: 08/11/2022
Pot Size: $60,500 USDC
Total HM: 6
Participants: 72
Period: 5 days
Judge: Picodes
Total Solo HM: 2
Id: 178
League: ETH
Rank: 21/72
Findings: 3
Award: $264.89
🌟 Selected for report: 0
🚀 Solo Findings: 0
151.3257 USDC - $151.33
There are three functions - https://github.com/code-423n4/2022-11-looksrare/blob/e3b2c053f722b0ca2dce3a3eb06f64859b8b7a6f/contracts/lowLevelCallers/LowLevelETH.sol#L32, https://github.com/code-423n4/2022-11-looksrare/blob/e3b2c053f722b0ca2dce3a3eb06f64859b8b7a6f/contracts/lowLevelCallers/LowLevelETH.sol#L43 and https://github.com/code-423n4/2022-11-looksrare/blob/e3b2c053f722b0ca2dce3a3eb06f64859b8b7a6f/contracts/lowLevelCallers/LowLevelETH.sol#L54 which are sending ETH back to user in different ways. If the user would appear to be a contract without receive() external payable or fallback() external payable functions then ETH would be stuck on the Aggregator contract and user won't receive any
It's recommended to check status variable after performing call - https://github.com/code-423n4/2022-11-looksrare/blob/e3b2c053f722b0ca2dce3a3eb06f64859b8b7a6f/contracts/lowLevelCallers/LowLevelETH.sol#L35, https://github.com/code-423n4/2022-11-looksrare/blob/e3b2c053f722b0ca2dce3a3eb06f64859b8b7a6f/contracts/lowLevelCallers/LowLevelETH.sol#L46 and https://github.com/code-423n4/2022-11-looksrare/blob/e3b2c053f722b0ca2dce3a3eb06f64859b8b7a6f/contracts/lowLevelCallers/LowLevelETH.sol#L57
#0 - c4-judge
2022-11-21T11:17:33Z
Picodes marked the issue as duplicate of #241
#1 - c4-judge
2022-12-16T13:56:41Z
Picodes marked the issue as satisfactory
77.2215 USDC - $77.22
LooksRareAggregator.execute() at the end of execution send all unspent ETH and ERC20 back to actor. But it send all amount lying on the contract which may be bigger than initially sent by actor. So anyone seeing that there are some ETH or ERC20 tokens stuck on contract may just send with a call of ERC20EnabledLooksRareAggregator.execute() 1 token of each ERC20 with positive contract balance, some dummy trade order (which may fail) and isAtomic=false flag and get back all funds on contract in ETH and ERC20.
There are special functions to rescue stuck ETH and ERC20 (TokenRescuer.rescueERC20() and TokenRescuer.rescueETH()) which can be called only by owner. So obviously by business-logic anyone shouldn't be able to get stuck ETH and ERC20.
Save initial amount and spent amount of ETH and ERC20 during fulfillment of all orders. Return only unused part of initially sent funds.
#0 - c4-judge
2022-11-19T09:59:41Z
Picodes marked the issue as duplicate of #277
#1 - c4-judge
2022-12-16T13:55:59Z
Picodes changed the severity to 2 (Med Risk)
#2 - c4-judge
2022-12-16T13:56:00Z
Picodes marked the issue as satisfactory
🌟 Selected for report: RaymondFam
Also found by: 0x1f8b, 0x52, 0xSmartContract, 0xc0ffEE, 0xhacksmithh, 8olidity, Awesome, BClabs, Bnke0x0, Chom, Deivitto, Hashlock, IllIllI, Josiah, KingNFT, Nyx, R2, ReyAdmirado, Rolezn, SamGMK, Sathish9098, SinceJuly, V_B, Vadis, Waze, a12jmx, adriro, ajtra, aphak5010, bearonbike, bin, brgltd, carlitox477, carrotsmuggler, cccz, ch0bu, chaduke, datapunk, delfin454000, erictee, fatherOfBlocks, fs0c, horsefacts, jayphbee, ktg, ladboy233, pashov, perseverancesuccess, rbserver, ret2basic, tnevler, zaskoh
36.3434 USDC - $36.34
TokenRescuer.rescueERC20() is intended to rescue ERC20 tokens stuck in the contract. But it doesn't work correctly with tokens getting commission from sender (e.g. CacheGold). It tries to send exactly IERC20(currency).balanceOf(address(this)) - 1). There would not be enough balance for a commission, so any try to call TokenRescuer.rescueERC20() would revert and tokens would stuck forever.
Add amount parameter to TokenRescuer.rescueERC20(). Do not allow to use arbitrary ERC20 (use a whitelist).
#0 - Picodes
2022-11-19T12:40:27Z
There is a sort of whitelist as tokens needs to be approved in the aggregator, and I don't think this falls within High Risk as in the first place this function is to rescue stucked funds only and is not supposed to be used.
#1 - c4-judge
2022-11-19T12:40:37Z
Picodes changed the severity to QA (Quality Assurance)
#2 - c4-sponsor
2022-11-24T12:29:27Z
0xhiroshi marked the issue as sponsor disputed
#3 - c4-judge
2022-12-12T10:05:11Z
Picodes marked the issue as grade-b