code-423n4 / 2022-05-cally-findings

2 stars 0 forks source link

System wrapper can trustlessly eliminate fees #299

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-05-cally/blob/main/contracts/src/Cally.sol#L281-L286

Vulnerability details

Impact

Creating an external system wrapper can successfully eliminate the fees for both sides. Such wrapper isn't difficult to create - in fact, it's a simple NFT marketplace.

The severity is high since it can be considered a fund theft - the fee collector has created the marketplace contract and made it a place for offers to find buyers. Not rewarding them despite work done is indeed losing funds.

For reference, a partially similar issue centered on wrappers with judged high severity can be found here.

Proof of Concept

Let Alice create a call. Bob purchased it with 0.1 ETH premium and 100 ETH strike price.

Bob decides they want to exercise the option. He needs to pay 100 ETH to get the underlying. Let the fee in the time he wants to exercise be 2%.

If he exercises, Alice will get additional 98 ETH. They can do better.

Alice knows she can get 98 ETH if Bob exercises. Thus, she may place the NFT of her vault on the marketplace for 99 ETH with time limit being Bob's option expiry.

Bob then can purchase that NFT expressing underlying from her for 99 ETH. He pays 1 ETH less, she gets 1 ETH more. No fees are paid.

Tools Used

Manual analysis

Recommended Mitigation Steps

Do not allow for vault transfers during an active option on that vault. If someone wraps from the beginning, it can be considered using another option marketplace and not fund theft.

outdoteth commented 2 years ago

this is technically true but is not an issue. As can be seen with various market places that charge fees, users still stick around (opensea etc.)

HardlyDifficult commented 2 years ago

This is a creative way to avoid fees and some may take advantage of that. However this seems to be more of a design consideration than an issue. The recommended mitigation imposes a limitation on other users - so there's a tradeoff decision to make there. And as the sponsor pointed out, many are willing to pay the fees for other conveniences - such as an easy to use interface.

Lowing to a (1) Low. Grouping this with the warden’s QA report, https://github.com/code-423n4/2022-05-cally-findings/issues/296