Platform: Code4rena
Start Date: 29/06/2022
Pot Size: $50,000 USDC
Total HM: 20
Participants: 133
Period: 5 days
Judge: hickuphh3
Total Solo HM: 1
Id: 142
League: ETH
Rank: 25/133
Findings: 2
Award: $631.05
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: zishansami
Also found by: 0x52, 0xsanson, auditor0517, berndartmueller, csanuragjain, zzzitron
Unfair and inconsistent fees
When exercising a put option, the full order.strike is transferred to the exerciser in L451. No fee is taken at any point during fillOrder and no fee is taken here meaning that not fee is taken at all on exercised put options.
Additionally when an expired unexercised put is withdrawn (i.e. !order.isCall && !isExercised) a fee is taken in L503. This is exact opposite of when the fee is intended to be taken.
L498 should be changed to:
if (fee > 0 && !isExercised)
L451 should be replaced with:
uint256 feeAmount = 0; if (fee > 0) { feeAmount = (order.strike * fee) / 1000; ERC20(order.baseAsset).safeTransfer(owner(), feeAmount); }
ERC20(order.baseAsset).safeTransfer(msg.sender, order.strike - feeAmount);
#0 - outdoteth
2022-07-06T18:23:48Z
Duplicate: Fees are only applied on puts if they are expired: https://github.com/code-423n4/2022-06-putty-findings/issues/269
🌟 Selected for report: xiaoming90
Also found by: 0x1f8b, 0x29A, 0x52, 0xDjango, 0xNazgul, 0xNineDec, 0xSolus, 0xf15ers, 0xsanson, AmitN, Bnke0x0, BowTiedWardens, Chom, David_, ElKu, Funen, GalloDaSballo, GimelSec, Hawkeye, IllIllI, JC, JohnSmith, Kaiziron, Kenshin, Lambda, Limbooo, MadWookie, Metatron, MiloTruck, Nethermind, Picodes, ReyAdmirado, Sneakyninja0129, StErMi, TomJ, Treasure-Seeker, TrungOre, Waze, Yiko, _Adam, __141345__, antonttc, async, aysha, catchup, cccz, cryptphi, csanuragjain, danb, datapunk, defsec, delfin454000, dirk_y, doddle0x, durianSausage, exd0tpy, fatherOfBlocks, gogo, hake, hansfriese, horsefacts, hubble, itsmeSTYJ, joestakey, oyc_109, pedroais, peritoflores, rajatbeladiya, reassor, robee, rokinot, samruna, saneryee, sashik_eth, shenwilly, shung, simon135, sseefried, unforgiven, zer0dot, zzzitron
47.1302 USDC - $47.13
Transaction reverts due to out of gas error because whitelist is too long
L670 simply loops through the entire list of whitelisted addresses until it matches msg.sender. If list is long and msg.sender is further down on the list, out of gas errors could easily happen. Additionally this could be use maliciously on counter offers to cause other user high gas fees.
Require the length of the whitelist to be less than some max length (i.e. 64) when calling fillOrder()
require(order.whitelist.length < 64)
#0 - outdoteth
2022-07-07T14:08:00Z
Any out of gas error here will show up in the user's wallet as an error. This will prevent them from wasting gas and submitting the tx to get a revert on chain. Technically this is a valid finding, but think it should be marked as low severity.
#1 - HickupHH3
2022-07-13T07:12:52Z
there's control over the whitelist off-chain. it can therefore be enforced to be of reasonable length or trimmed if necessary. non-crit IMO.
warden has no QA report, this issue will be the primary