code-423n4 / 2022-06-putty-findings

5 stars 0 forks source link

An attacker can force a flash loan and get temporary ownership of any NFT held inside putty and perform arbitrary logic with it. #375

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-06-putty/blob/3b6b844bc39e897bd0bbb69897f2deff12dc3893/contracts/src/PuttyV2.sol#L303 https://github.com/code-423n4/2022-06-putty/blob/3b6b844bc39e897bd0bbb69897f2deff12dc3893/contracts/src/PuttyV2.sol#L324 https://github.com/code-423n4/2022-06-putty/blob/3b6b844bc39e897bd0bbb69897f2deff12dc3893/contracts/src/PuttyV2.sol#L442 https://github.com/code-423n4/2022-06-putty/blob/3b6b844bc39e897bd0bbb69897f2deff12dc3893/contracts/src/PuttyV2.sol#L646

Vulnerability details

Impact

An attacker can force a flash loan and get temporary ownership of any NFT held inside putty and perform arbitrary logic with it.

Proof of Concept

A reentrancy gate can be exploited to provoke an unintended flash loan of any NFT held inside the contract.

Initial state : The putty contract is holding an NFT (from now on called "real NFT") from a legitimate option opened by regular users. An attacker can use reentrancy to get a flash loan of "real NFT" and perform arbitrary transactions with ownership of that NFT before giving it back (in the same transaction).

Attack :

An attacker makes and fulfills a fake order (he is the maker and the taker) with a base asset (the asset in which the premium and strike are paid) equal to an attacker contract. The base asset isn't a token but a contract that can perform arbitrary logic.

The order is a long call with a strike and premium of 0 for the NFT that's inside the contract. The erc721Assets[] array in the order includes the real NFT to be flashloaned and then a fake NFT allowing to do another external call. The attacker doesn't have the real NFT (it's inside the contract). The goal of the attack is to get a flash loan on that NFT and perform arbitrary logic with it with the goal of getting to claim any rewards or funds claimable by the real NFT. The erc721Assets[] array is composed of [realNFTAdddress, fakeNFTaddress]

Attack flow : -The attacker calls fillOrder() with the order made by himself. Fill order first mints the option NFT and then makes the base asset transfer. https://github.com/code-423n4/2022-06-putty/blob/3b6b844bc39e897bd0bbb69897f2deff12dc3893/contracts/src/PuttyV2.sol#L303 https://github.com/code-423n4/2022-06-putty/blob/3b6b844bc39e897bd0bbb69897f2deff12dc3893/contracts/src/PuttyV2.sol#L324

-This base asset transfer is the first reentrancy gate. The base asset is an attacker contract that will immediately call the exercise() function (the attacker is the maker and taker so he can do this with contracts). Note that the real NFT still didn't have to get transfered in since the base asset transfer happens before that.

-The exercise function sets exercised to true, burns the position and transfers the real NFT out. https://github.com/code-423n4/2022-06-putty/blob/3b6b844bc39e897bd0bbb69897f2deff12dc3893/contracts/src/PuttyV2.sol#L442

-The attacker now has ownership of the real NFT -Then the fake NFT is transferred out in the function loop which handles execution back to the attacker. At this point, he can perform arbitrary logic with the real NFT including cashing out on anything that requires being the owner of the NFT. This attack is only useful with NFTs that provide access to funds (this is why the issue is medium and not high severity).

https://github.com/code-423n4/2022-06-putty/blob/3b6b844bc39e897bd0bbb69897f2deff12dc3893/contracts/src/PuttyV2.sol#L646

After the arbitrary logic is performed by the attacker. The execution comes back to fillOrder() and assets have to be transferred inside the contract again, ending the forced flash loan. https://github.com/code-423n4/2022-06-putty/blob/3b6b844bc39e897bd0bbb69897f2deff12dc3893/contracts/src/PuttyV2.sol#L376

To recap the flow : The attacker calls fillOrder() and reenters the contract in the baseAssetTransfer by calling exercise() which means the NFT is transferred out before being transferred in. The real NFT is transferred out. The fake NFT transfer external call is used to perform arbitrary logic. Then the execution comes back to fillOrder() and the NFT is transferred into putty again.

The attack has no cost besides gas since both premium and strike are paid in the fake attacker contract.

Recommended Mitigation Steps

Add reentrancy protection

GalloDaSballo commented 2 years ago

"-The exercise function sets exercised to true, burns the position and transfers the real NFT out."

If fillOrder is being exited during the premium-transfer, the NFT is still not in the contract, meaning the "flashborrow" would revert at the last part of exercise because PuttyV2 still has to receive the NFT tokens.

Also, this would require a user that is wiling to be the counter-party to a fake token

Pedroais commented 2 years ago

"-The exercise function sets exercised to true, burns the position and transfers the real NFT out."

If fillOrder is being exited during the premium-transfer, the NFT is still not in the contract, meaning the "flashborrow" would revert at the last part of exercise because PuttyV2 still has to receive the NFT tokens.

Also, this would require a user that is wiling to be the counter-party to a fake token

Hey Alex ! I think your points are not true 1 - the NFT was already in the contract, that's the idea of the attack, to make a flashborrow on an NFT that's already inside the contract (from an unrelated legitimate position). Transfer out is made before transfer in and won't revert since the contract was already holding the nft. Then the attacker uses the nft and transfers it back in.

2 - The attacker can make an order and take it himself so a counterparty is not needed

I actually sent a help request to edit the submission but it wasn't responded since the team is on holiday. The fake token isn't actually needed since the external call of ERC721 safe transfer can be used to allow the attacker to perform arbitrary logic on the NFT. The issue is still correct as described but could be a bit simpler.

So to recap lets use BAYC as an example:

BAYC 1 is inside the contract and an attacker wants to get a flash loan on it

-Attacker makes order with himself and reenters in base transfer to exercise -BAYC 1 is transfered out before in and it doesn't revert since it's already inside putty -The attacker performs arbitrary logic on the BAYC using the ERC721 safe transfer -Execution comes back to fillOrder() and the attacker returns the BAYC

GalloDaSballo commented 2 years ago

I was wrong, finding is valid: -> fillOrder -> premium transfer execution hijack -> exercise -> Receive any NFT inside the contract -> Do w/e -> Execution returns to exercise

Because fee token can be the malicious token, this is a free flashloan

outdoteth commented 2 years ago

Duplicate: It’s possible to flashloan all assets in the contract without paying a protocol fee: https://github.com/code-423n4/2022-06-putty-findings/issues/377