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: 4/133
Findings: 6
Award: $2,137.90
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: zishansami
Also found by: 0x52, 0xsanson, auditor0517, berndartmueller, csanuragjain, zzzitron
583.9233 USDC - $583.92
https://github.com/code-423n4/2022-06-putty/blob/main/contracts/src/PuttyV2.sol#L466
For a long PUT condition the party which is doing short will always be in loss
Alice creates and signs a long put option order off-chain for 2 Bored Ape floors with a duration of 30 days, a strike of 124 WETH and a premium of 0.8 WETH
Bob takes Alice's order and fills it by sumbitting it to the Putty smart contract using fillOrder()
He sends 124 ETH to cover the strike which is converted to WETH. 0.8 WETH is transferred from Alice's wallet to Bob's wallet.
A long NFT is sent to Alice and a short NFT is sent to Bob which represents their position in the trade
Now 2 cases:
Case 1: a. 17 days pass and the floor price for Bored Apes has dropped to 54 ETH - (2 * 54 = 108 ETH. 124 - 108 = 16 ETH profit for Alice.) b. Alice exercise her option and Bob withdraws to get Bored Apes with loss of 16 ETH
Case 2: a. 17 days pass and the floor price for Bored Apes has increased to 90 ETH - (2 * 90 = 180 ETH. 124 - 180 = -56 ETH loss for Alice.) b. Of course Alice chose to lose 0.8 ETH premium and does nothing c. Since Alice does not exercise so Bob need to wait for order expiration and then withdraws the strike amount with 3% fees. This means Bob gets 124-(1.24*3) = 121 ETH which means Bob is at loss of 3 ETH
Implement some fees for long position as well once order is filled. The fees should be reimbursed once order is execised
#0 - GalloDaSballo
2022-07-05T01:38:46Z
It seems like the fees mechanism may cause negative incentives, that said I'd assume the premium would have been "realPremium + potentialFee".
However the example provided does make sense
#1 - 0xlgtm
2022-07-05T06:29:09Z
Can the sponsors confirm when fees should be taken from the strike?
#2 - outdoteth
2022-07-06T12:05:06Z
Fees should only be taken on exercise so peripherally this issue is correct because it highlights that.
However, the severity of this is not high because fees are expected part of the platform - any trading platform with fees has -EV for all traders. Traders having to pay fees is expected behaviour.
#3 - berndartmueller
2022-07-06T12:41:12Z
... Traders having to pay fees is expected behaviour.
That's true. But paying fees on the returned strike, due to a not exercised put option, does not make sense. In this case, fees should be paid on the earned premium.
#4 - outdoteth
2022-07-06T14:15:33Z
Ok sure, tbh I am not very sure on ranking the severity of this issue. Just that most issues which reported the “taking fees on expiry” were tagged as Med.
#5 - outdoteth
2022-07-06T18:30:54Z
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: kirk-baird
Also found by: Lambda, csanuragjain, hansfriese, minhquanym
https://github.com/code-423n4/2022-06-putty/blob/main/contracts/src/PuttyV2.sol#L573
acceptCounterOffer is not verifying if the original order has already been filled. In case maker makes a counter offer and by the time counter offer is called, some user has already filled the original order then both original and counter offer will be filled. Maker will unnecessary have 2 positions (when maker only wanted 1). This means if user was incurring loss on this order then the loss would get doubled
Add below check in acceptCounterOffer to verify if original order is not already filled
bytes32 orderHash = hashOrder(originalOrder); require(ownerOf(uint256(orderHash)) == address(0), "Original order already filled");
#0 - GalloDaSballo
2022-07-05T01:52:16Z
You cannot fillOrder of an order whose NFT was already minted, the tx will revert at _mint
#1 - csanuragjain
2022-07-05T05:44:27Z
@GalloDaSballo Correct but in this case, the counter order hash should differ from the original one. An example will be user created order O1 as PUT Long with premium 1. Later he created a counter order O2 with premium 6 to lure more users. In this case hash for both orders will differ and hence _mint should work as different NFT will be given for each order. Can you please suggest
#2 - GalloDaSballo
2022-07-05T14:25:33Z
The warden is showing that if you sign 2 orders:
The SC has no guarantee as to whether one, or both will be filled, OrderA could be filled, then Cancelled, then B Or Order B could be filled and A cancelled.
It seems like acceptCounterOffer is calling cancel which requires the maker to call it, but then calls fillOrder that requires the taker (msg.sender) to be someone else.
I think a judge + sponsor will have to figure this out
#3 - outdoteth
2022-07-06T18:08:21Z
can confirm that this is an issue
#4 - outdoteth
2022-07-06T18:09:05Z
Duplicate: It’s possible to fill an order twice by accepting a counter offer for an already filled order: https://github.com/code-423n4/2022-06-putty-findings/issues/44
🌟 Selected for report: kirk-baird
Also found by: 0xA5DF, Kenshin, cccz, chatch, csanuragjain, hansfriese, hyh, itsmeSTYJ, pedroais, sashik_eth, unforgiven, xiaoming90
https://github.com/code-423n4/2022-06-putty/blob/main/contracts/src/PuttyV2.sol#L268
Since both strike and premium amount can be zero, a malicious user can create order with malicious tokens which are free to grab
Ensure both strike amount and premium amount are greater than 0
#0 - GalloDaSballo
2022-07-05T01:32:48Z
Why would a user willingly accept a malicious order?
#1 - outdoteth
2022-07-07T13:34:16Z
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
#2 - HickupHH3
2022-07-09T02:33:54Z
Why would a user willingly accept a malicious order?
Because FOMO and... you know, problem between keyboard and chair :p
🌟 Selected for report: horsefacts
Also found by: 0xc0ffEE, IllIllI, Picodes, Sm4rty, berndartmueller, csanuragjain, shenwilly, unforgiven
35.1904 USDC - $35.19
https://github.com/code-423n4/2022-06-putty/blob/main/contracts/src/PuttyV2.sol#L303
While minting NFT there is no check to see whether receiving end is supporting ERC721 or not. In case if ERC721 is not supported, mint would be lost and user funds will be locked in PuttyV2 forever
Use safeMint function which will revert if receiving end does not implement ERC721 properly
#0 - outdoteth
2022-07-06T19:39:13Z
Duplicate: Contracts that can’t handle ERC721 tokens will lose their Putty ERC721 position tokens: https://github.com/code-423n4/2022-06-putty-findings/issues/327
🌟 Selected for report: hyh
Also found by: Treasure-Seeker, csanuragjain, minhquanym
https://github.com/code-423n4/2022-06-putty/blob/main/contracts/src/PuttyV2.sol#L268
It is not checked if the order premium is set to 0. Attacker can use it to defame the protocol and force victim to lose funds An attacker can set premium as 0 and set a very attractive strike amount. Once victim fills order with this strike amount, attacker (long put) never exercises. Victim post long position expiration gets back his fund - fees
Alice creates a long put order with premium as 0 and strike price as 10 eth
Bob finds strike price to be really good for this order and ignores the 0 premium
Bob fills the order and provide strike amount to contract
Alice who wanted to defame the putty v2 protocol, simply never exercises
This forces Bob to withdraw after order expiry and get his strike amount - fees which cause loss of funds
Add a check to see that
require(order.premium>0, "Zero premium");
#0 - outdoteth
2022-07-07T13:27:03Z
The core of this issue is not valid because it is essentially saying "people can create underpriced options".
But peripherally it still highlights the following issue: Duplicate: Charging fees on the strike amount instead of the premium amount can lead to disproportionate fees: https://github.com/code-423n4/2022-06-putty-findings/issues/373
so marking this as a duplicate.
🌟 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.1318 USDC - $47.13
https://github.com/code-423n4/2022-06-putty/blob/main/contracts/src/PuttyV2.sol#L1
The contract should prohibit burn function which is not required. If any user accidentally burns there NFT then they will loss funds (strike amount)