No approve before transfer

[code423n4]

Lines of code

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

Vulnerability details

No approve before transfer

Proof of Concept

There are multiple transfer without approve upfront.

324:
            ERC20(order.baseAsset).safeTransferFrom(order.maker, msg.sender, order.premium);
336:
                IWETH(weth).transfer(order.maker, msg.value);
338:
                ERC20(order.baseAsset).safeTransferFrom(msg.sender, order.maker, order.premium);
344:
            ERC20(order.baseAsset).safeTransferFrom(order.maker, address(this), order.strike);
360:
                ERC20(order.baseAsset).safeTransferFrom(msg.sender, address(this), order.strike);
368-369:
            _transferERC20sIn(order.erc20Assets, order.maker);
            _transferERC721sIn(order.erc721Assets, order.maker);
375-377:
            _transferERC20sIn(order.erc20Assets, msg.sender);
            _transferERC721sIn(order.erc721Assets, msg.sender);
            _transferFloorsIn(order.floorTokens, floorAssetTokenIds, msg.sender);
413:
        transferFrom(msg.sender, address(0xdead), uint256(orderHash));
436:
                ERC20(order.baseAsset).safeTransferFrom(msg.sender, address(this), order.strike);
440-442:
            _transferERC20sOut(order.erc20Assets);
            _transferERC721sOut(order.erc721Assets);
            _transferFloorsOut(order.floorTokens, positionFloorAssetTokenIds[uint256(orderHash)]);
451-456:
            ERC20(order.baseAsset).safeTransfer(msg.sender, order.strike);

            // transfer assets from exerciser to putty
            _transferERC20sIn(order.erc20Assets, msg.sender);
            _transferERC721sIn(order.erc721Assets, msg.sender);
            _transferFloorsIn(order.floorTokens, floorAssetTokenIds, msg.sender);
488:
        transferFrom(msg.sender, address(0xdead), uint256(orderHash));
500:
                ERC20(order.baseAsset).safeTransfer(owner(), feeAmount);
503:
            ERC20(order.baseAsset).safeTransfer(msg.sender, order.strike - feeAmount);
510-511:
            _transferERC20sOut(order.erc20Assets);
            _transferERC721sOut(order.erc721Assets);
516:
            _transferFloorsOut(order.floorTokens, positionFloorAssetTokenIds[floorPositionId]);
601:
            ERC20(token).safeTransferFrom(from, address(this), tokenAmount);
612:
            ERC721(assets[i].token).safeTransferFrom(from, address(this), assets[i].tokenId);
628:
            ERC721(floorTokens[i]).safeTransferFrom(from, address(this), floorTokenIds[i]);
638:
            ERC20(assets[i].token).safeTransfer(msg.sender, assets[i].tokenAmount);
648:
            ERC721(assets[i].token).safeTransferFrom(address(this), msg.sender, assets[i].tokenId);
659:
            ERC721(floorTokens[i]).safeTransferFrom(address(this), msg.sender, floorTokenIds[i]);

Impact

Failure to execute the transfer.

Tools Used

Mannual analysis.

Recommended Mitigation Steps

Add the approve steps.

[ecmendenhall]

Some of these examples are transfer calls that do not require an approval. In the others, the assumption is that approvals have been set by the caller.