DittoETH

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

TransferFrom from ERC721Facet does not ensure short position is healthy

Summary

If a shorter is in a really bad situation (CR <= 1.1) he/she knows he will be liquidated and will lose all it's collateral but he/she still has the option to transfer it's NFT to someone else and liquidate it immediately to gain callerFee

Vulnerability Details

ERC721Facet.sol :

function transferFrom(address from, address to, uint256 tokenId) public {
// @dev ensure the tokenId can be downcasted to 40 bits
if (tokenId > type(uint40).max) revert Errors.InvalidTokenId();
if (
msg.sender != from && !s.isApprovedForAll[from][msg.sender] && msg.sender != s.getApproved[tokenId]
) revert Errors.ERC721InsufficientApproval(msg.sender, tokenId);
address owner = ownerOf(tokenId);
if (owner != from) {
revert Errors.ERC721IncorrectOwner(from, tokenId, owner);
}
if (to == address(0)) {
revert Errors.ERC721InvalidReceiver(address(0));
}
STypes.NFT memory nft = s.nftMapping[tokenId];
address asset = s.assetMapping[nft.assetId];
//@dev If NFT does not exist, ERC721NonexistentToken() will trigger
LibShortRecord.transferShortRecord(asset, from, to, uint40(tokenId), nft);
delete s.getApproved[tokenId];
emit Events.Transfer(from, to, tokenId);
}

As you can see there is no restrictions to transfer a short so any shorter in a really bad situation could transfer it's short order NFT position to another address.

Moreover if a short has a CR less than 1.1 (minimumCR) he can gains something from doing this.
Looking at the documentation we can see this :

CR Margin Caller Shorter Pool Description
CR >= 4 n/a n/a n/a n/a
1.1 <= CR < 4 callerFee c - c2 tappFee shorter gets remaining collat
CR <= 1.1 callerFee 0 c - c2 + tappFee pool gets remaining collat

In any case the liquidation succeeds the liquidator will receive callerFee, so a bad position shorter would be incentivize to transfer it's short to another address and liquidate it.

Impact

Short with a really bad CR (<1.1) can be transferred and instantly liquidated

Tools Used

VSCode

Recommendations

Don't allow a short to be transferred if CR is less than 4

Updates

Lead Judging Commences

0xnevi Lead Judge
almost 2 years ago
0xnevi Lead Judge almost 2 years ago
Submission Judgement Published
Invalidated
Reason: Other

Support

FAQs

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