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: 21/133
Findings: 5
Award: $827.89
🌟 Selected for report: 1
🚀 Solo Findings: 0
🌟 Selected for report: kirk-baird
Also found by: 0xA5DF, Kenshin, cccz, chatch, csanuragjain, hansfriese, hyh, itsmeSTYJ, pedroais, sashik_eth, unforgiven, xiaoming90
41.8933 USDC - $41.89
The malicious user can use any tokens as order.erc20Assets when creating an order, which can be fee-on-transfer and rebase tokens or tokens created by the malicious user himself, which will prevent other users from exercising their options or withdrawing their assets.
https://github.com/code-423n4/2022-06-putty/blob/3b6b844bc39e897bd0bbb69897f2deff12dc3893/contracts/src/PuttyV2.sol#L368 https://github.com/code-423n4/2022-06-putty/blob/3b6b844bc39e897bd0bbb69897f2deff12dc3893/contracts/src/PuttyV2.sol#L454
None
Consider adding a whitelist to order.erc20Assets
#0 - outdoteth
2022-07-07T13:36:11Z
Duplicate: Setting malicious or invalid erc721Assets, erc20Assets or floorTokens prevents the option from being exercised: https://github.com/code-423n4/2022-06-putty-findings/issues/50
🌟 Selected for report: IllIllI
Also found by: 0x29A, 0xDjango, 0xc0ffEE, AmitN, BowTiedWardens, StErMi, auditor0517, berndartmueller, cccz, danb, dipp, dirk_y, hansfriese, horsefacts, hyh, kirk-baird, oyc_109, peritoflores, rfa, sseefried, swit, xiaoming90, zzzitron
5.5216 USDC - $5.52
In fillOrder and exercise functions, when the caller needs to transfer ERC20 tokens, there is no check msg.value == 0. When the caller accidentally carries Ether in the transaction, the caller will lost his Ether.
https://github.com/code-423n4/2022-06-putty/blob/3b6b844bc39e897bd0bbb69897f2deff12dc3893/contracts/src/PuttyV2.sol#L338-L339 https://github.com/code-423n4/2022-06-putty/blob/3b6b844bc39e897bd0bbb69897f2deff12dc3893/contracts/src/PuttyV2.sol#L360-L361 https://github.com/code-423n4/2022-06-putty/blob/3b6b844bc39e897bd0bbb69897f2deff12dc3893/contracts/src/PuttyV2.sol#L436-L437
None
When the caller transfers ERC20 tokens, check msg.value == 0.
#0 - rotcivegaf
2022-07-04T23:43:29Z
Duplicate of #226
#1 - outdoteth
2022-07-06T19:28:11Z
Duplicate: Native ETH can be lost if it’s not utilised in exercise and fillOrder: https://github.com/code-423n4/2022-06-putty-findings/issues/226
🌟 Selected for report: cccz
Also found by: IllIllI, minhquanym
622.994 USDC - $622.99
An attacker can create a short put option on cryptopunk. When the user fulfills the order, the baseAsset will be transferred to the contract. However, since cryptopunk does not support ERC721, the user cannot exercise the option because the safeTransferFrom function call fails. Attacker can get premium and get back baseAsset after option expires
https://github.com/code-423n4/2022-06-putty/blob/3b6b844bc39e897bd0bbb69897f2deff12dc3893/contracts/src/PuttyV2.sol#L343-L346 https://github.com/code-423n4/2022-06-putty/blob/3b6b844bc39e897bd0bbb69897f2deff12dc3893/contracts/src/PuttyV2.sol#L628-L629
None
Consider adding a whitelist to nfts in the order, or consider supporting exercising on cryptopunk.
#0 - 0xlgtm
2022-07-05T09:17:10Z
Putty uses solmate's ERC721.safeTransferFrom which requires that the NFT contract implements onERC721Received. For the case of OG NFTs like punks and rocks, this will fail, https://github.com/Rari-Capital/solmate/blob/main/src/tokens/ERC721.sol#L120
#1 - thereksfour
2022-07-06T08:01:15Z
The user does not need to send cryptopunk to the contract when fulfilling the short put option order, but the user will pay a premium to the order creator. Later, when the user wants to exercise the option, since the cryptopunk does not support safetransferfrom, the user cannot exercise the option.
#2 - 0xlgtm
2022-07-06T10:02:03Z
The user does not need to send cryptopunk to the contract when fulfilling the short put option order, but the user will pay a premium to the order creator. Later, when the user wants to exercise the option, since the cryptopunk does not support safetransferfrom, the user cannot exercise the option.
Sorry, I did not consider this path. You are correct to say that a maker can create a short put option order with cryptopunks as a token and the holder of the long put option will not be able to exercise since cryptopunks cannot be transferred with safeTransferFrom. From that perspective, this is a valid issue. Thank you for bringing it up. I will defer to the judge for the final decision.
#3 - outdoteth
2022-07-08T12:18:47Z
we dont intend to support cryptopunks or cryptokitties. if users wish to use these tokens then they can get wrapped versions (ex: wrapped cryptopunks)
#4 - HickupHH3
2022-07-12T13:29:40Z
I thought cryptokitties are ERC721? I think they were the ones who popularized the standard actually :p Probably meant etherrocks.
In general, non-compliant ERC-721 NFTs can be supported through wrappers, though some users might be unaware... Downgrading to med severity, similar to this issue from another contest.
🌟 Selected for report: codexploder
Also found by: ACai, Critical, cccz, horsefacts, ignacio, shenwilly, unforgiven, xiaoming90
The fillOrder function does not check whether order.duration is greater than a minimum value. If a malicious user creates a short option order with order.duration = 0, the malicious attacker can call the withdraw function immediately after the user fulfills the order.
None
order.duration should be checked for values greater than a minimum in fillOrder function
#0 - outdoteth
2022-07-06T19:44:19Z
Duplicate: Orders with low durations can be easily DOS’d and prevent possibility of exercise: https://github.com/code-423n4/2022-06-putty-findings/issues/265
🌟 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
ORDER_TYPE_HASH is incorrect, ")" should be at the end
None
bytes32 public constant ORDER_TYPE_HASH = keccak256( abi.encodePacked( "Order(", "address maker,", "bool isCall,", "bool isLong,", "address baseAsset,", "uint256 strike,", "uint256 premium,", "uint256 duration,", "uint256 expiration,", "uint256 nonce,", "address[] whitelist,", "address[] floorTokens,", "ERC20Asset[] erc20Assets,", "ERC721Asset[] erc721Assets", - ")", "ERC20Asset(address token,uint256 tokenAmount)", "ERC721Asset(address token,uint256 tokenId)", + ")", ) );