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: 95/133
Findings: 1
Award: $47.13
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 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
User shouldn't be able to fill an order with a strike = 0 or premium = 0, as it creates inconsistencies in the code when handling native ETH.
For example in fillOrder()
https://github.com/code-423n4/2022-06-putty/blob/main/contracts/src/PuttyV2.sol#L322-L340
// transfer premium to whoever is short from whomever is long if (order.isLong) { ERC20(order.baseAsset).safeTransferFrom(order.maker, msg.sender, order.premium); } else { // handle the case where the user uses native ETH instead of WETH to pay the premium if (weth == order.baseAsset && msg.value > 0) { // check enough ETH was sent to cover the premium require(msg.value == order.premium, "Incorrect ETH amount sent"); // convert ETH to WETH and send premium to maker // converting to WETH instead of forwarding native ETH to the maker has two benefits; // 1) active market makers will mostly be using WETH not native ETH // 2) attack surface for re-entrancy is reduced IWETH(weth).deposit{value: msg.value}(); IWETH(weth).transfer(order.maker, msg.value); } else { ERC20(order.baseAsset).safeTransferFrom(msg.sender, order.maker, order.premium); } }
A user is allowed to fill the order with premium 0 and if the order.baseAsset is weth, because the msg.value has to be grater than 0, it's going to skip that and execute the ERC20 transfer instead.
I found 3 instances of that issue:
fillOrder()
https://github.com/code-423n4/2022-06-putty/blob/main/contracts/src/PuttyV2.sol#L322-L340excercise()
In order to fix that just add a check in the fillOrder()
to restrict strike and premiums with 0 value.
require(order.strike > 0 && order.premium > 0)
#0 - HickupHH3
2022-07-15T02:53:42Z
WETH doesnt revert on 0 amount transfers, so it's fine.