Platform: Code4rena
Start Date: 28/11/2022
Pot Size: $192,500 USDC
Total HM: 33
Participants: 106
Period: 11 days
Judge: LSDan
Total Solo HM: 15
Id: 186
League: ETH
Rank: 31/106
Findings: 2
Award: $835.72
🌟 Selected for report: 0
🚀 Solo Findings: 0
731.7962 USDC - $731.80
If a customer sets canFail to true for a X2Y2 order and the order fails, the ETH is not refunded to them..
customer sets canFail to true customer calls function An order fails X2Y2 sends ETH back to PoolMarketplace PoolMarketplace doesn’t refund the customer.
Prevent customers from setting canFail to true in the X2Y2 adapter.
#0 - c4-judge
2022-12-20T17:08:44Z
dmvt marked the issue as duplicate of #34
#1 - c4-judge
2023-01-23T20:46:07Z
dmvt marked the issue as satisfactory
🌟 Selected for report: IllIllI
Also found by: 0x4non, 0x52, 0xAgro, 0xNazgul, 0xSmartContract, 0xackermann, 9svR6w, Awesome, Aymen0909, B2, BRONZEDISC, Bnke0x0, Deekshith99, Deivitto, Diana, Dravee, HE1M, Jeiwan, Kaiziron, KingNFT, Lambda, Mukund, PaludoX0, RaymondFam, Rolezn, Sathish9098, Secureverse, SmartSek, __141345__, ahmedov, ayeslick, brgltd, cccz, ch0bu, chrisdior4, cryptonue, cryptostellar5, csanuragjain, datapunk, delfin454000, erictee, gz627, gzeon, helios, i_got_hacked, ignacio, imare, jadezti, jayphbee, joestakey, kankodu, ksk2345, ladboy233, martin, nadin, nicobevi, oyc_109, pashov, pavankv, pedr02b2, pzeus, rbserver, ronnyx2017, rvierdiiev, shark, unforgiven, xiaoming90, yjrwkk
103.9175 USDC - $103.92
An operator can grief PoolMarketplace by draining its WETH into X2Y2
In its run function X2Y2 checks if input.shared.amountToEth is greater than 0. If it is, WETH is moved from msg.sender to X2Y2 with amountToEth is set to the amount of WETH unwrapped. amountToEth is the result of amountToEth minus the uint256 returned by the _run function. The X2Y2 adapter doesn’t check input.shared.amountToEth.
What is the currency set to? DAI, in this case
Why? When the _run function is called it returns an amount which is subtracted from and saved to amountToEth.
Why does this matter? When amountToEth is 0 the X2Y2 contract will not send any ETH back.
What does this mean? When an operator uses calls the X2Y2 contract via PoolMarketplace setting his currency to an accepted ERC20, DAI, and input.shared.amountToEth to a non zero amount, WETH is moved from PoolMarketplace to the X2Y2 contract in the specified amount because PoolMarketplace is the msg.sender. Because currency is set to DAI, in this case, it is moved from PoolMarketplace to X2Y2 and saved to the nativeAmount which is used to determine amountToEth.
Succinctly? This bug allows an operator to grief the PoolMarketplace contract. It does this by setting input.shared.amountToEth to the amount of WETH in the contract, setting currency to an accepted ERC20 that isn’t WETH and setting amount to equal PoolMarketplaces’ total WETH amount.
PoolMarketplace has 50 WETH Operator creates NFT Operator sets NFTs price to 50 DAI Operator calls buyWithCredit using X2Y2 adapter with currency set to DAI and with input.shared.amountToEth = 50 WETH When the run function within X2Y2 is called 50 WETH is pulled from PoolMarketplace into X2Y2 setting amountEth to 50. amountEth is set to 0 via the _takePayment function which transfers the 50 DAI to the X2Y2 setting nativeAmount to 50. When the _run function returns it returns 50, 50 - 50, setting amountEth to 0. 50 DAI is paid to the operator less fees PoolMarketplace isn’t refunded it’s ETH because amountEth == 0.
Require input.shared.amountToEth && input.shared.amountToWeth == 0.
#0 - c4-judge
2022-12-20T18:05:08Z
dmvt marked the issue as duplicate of #54
#1 - c4-judge
2023-01-23T15:43:57Z
dmvt marked the issue as satisfactory
#2 - c4-judge
2023-01-31T17:30:20Z
dmvt marked the issue as not a duplicate
#3 - c4-judge
2023-01-31T17:30:34Z
dmvt changed the severity to QA (Quality Assurance)
#4 - c4-judge
2023-01-31T17:30:49Z
dmvt marked the issue as grade-b
#5 - dmvt
2023-01-31T17:33:57Z
There's really no reason for anyone to ever do this.