Lines of code

Vulnerability details

Impact

when fillOrder() is called code mints two PuttyV2 NFT token, one for Long position and one for Short Position and It’s possible to transfer this NFT tokens to others. exercising unwanted bad Long positions can cause users to lose funds and tokens, for example if someone exercise Long CALL BTC at $100K or Long PUT BTC at $1K s/he would lose lots of funds, so attacker can create lots of Long positions NFTs(he can sign short orders which order.baseAsset is some worthless token and then call fillOrder() to create Long position NFT) and then transfer them to all protocol users (in one transaction to save gas) and users would see them in the UI and if one user by mistake clicks on exercise() (attacker can even create malicious positions based on each user balances and trick them to click on exercise) he would lose his funds because contract would transfer user funds and in return user receive some worthless order.baseAsset tokens.

Proof of Concept

This is transferFrom() code in PuttyV2NFT contract:

    // remove balanceOf modifications
    function transferFrom(
        address from,
        address to,
        uint256 id
    ) public override {
        require(from == _ownerOf[id], "WRONG_FROM");
        require(to != address(0), "INVALID_RECIPIENT");
        require(
            msg.sender == from || isApprovedForAll[from][msg.sender] || msg.sender == getApproved[id],
            "NOT_AUTHORIZED"
        );

        _ownerOf[id] = to;
        delete getApproved[id];

        emit Transfer(from, to, id);
    }

As you can see it allows to transfer NFT position tokens to others. attacker can use this to create a lots of malicious Long positions and transfer them to other users and trick users (or users by mistake) to click on exercise() and steal users funds.
To exploit this attacker would look at balance of each user of protocol and create malicious Long positions like Long CALL BTC at $100K or Long PUT BTC at $1K (based on user ERC20 or ERC721 token balances) and transfer them to users. To create Long positions attacker first signs the malicous short position with one of his wallets and set baseAsset to some worthless asset or set Premium to 0 and then by calling fillOrder() with his another wallet attacker would create a lot of Long positions (he can do this in one transaction to save gas) then he would transfer position NFT tokens to others and trick them to click on exercise() on UI (or some user click on it by mistake because of huge number of malicious long NFT positions so some users would do this mistake) and then contract would transfer user funds to contract address and give user some worthless tokens and user would lose his funds and attacker can claim user funds by calling withdraw() for the opposite position(short position).

Tools Used

VIM

Recommended Mitigation Steps

Transferring Long positions shouldn’t be allowed because exercising bad long position can cause users to lose funds.

The text was updated successfully, but these errors were encountered:

👎 3 kirk-baird, csanuragjain, and 0xlgtm reacted with thumbs down emoji

All reactions

• 👎 3 reactions