Blur Exchange contest - jayphbee's results

Lines of code

https://github.com/code-423n4/2022-10-blur/blob/main/contracts/matchingPolicies/StandardPolicyERC1155.sol#L33 https://github.com/code-423n4/2022-10-blur/blob/main/contracts/matchingPolicies/StandardPolicyERC1155.sol#L59 https://github.com/code-423n4/2022-10-blur/blob/main/contracts/BlurExchange.sol#L145

Vulnerability details

Impact

Buyer can receive less ERC1155 tokens than expected but pay too much.

Proof of Concept

1. Alice sells ERC1155 token with tokenId 1 and amount 10 for total price 10 ether.

2. Bob matches Alice's order paying 10 ether and expects to receive 10 tokenId 1.

3. Bob actually only get 1 tokenId 1 but paid 10 ether to Alice.

This is because _canMatchOrders always returns amount = 1 for ERC1155 tokens

(uint256 price, uint256 tokenId, uint256 amount, AssetType assetType) = _canMatchOrders(sell.order, buy.order);

due to

https://github.com/code-423n4/2022-10-blur/blob/main/contracts/matchingPolicies/StandardPolicyERC1155.sol#L33 https://github.com/code-423n4/2022-10-blur/blob/main/contracts/matchingPolicies/StandardPolicyERC1155.sol#L59

That is to say Bob pays the total price(10 ether) when calling _executeFundsTransfer and receive for 1 amount when calling _executeTokenTransfer which leads to Bob lose 9 ether.

Tools Used

Manual review

Recommended Mitigation Steps

canMatchMakerAsk and canMatchMakerBid should return the order said amount of tokens.

contract StandardPolicyERC1155 is IMatchingPolicy {
    function canMatchMakerAsk(Order calldata makerAsk, Order calldata takerBid)
        external
        pure
        override
        returns (
            bool,
            uint256,
            uint256,
            uint256,
            AssetType
        )
    {
        return (
            (makerAsk.side != takerBid.side) &&
            (makerAsk.paymentToken == takerBid.paymentToken) &&
            (makerAsk.collection == takerBid.collection) &&
            (makerAsk.tokenId == takerBid.tokenId) &&
            (makerAsk.matchingPolicy == takerBid.matchingPolicy) &&
            (makerAsk.price == takerBid.price) &&
            (makerAsk.amount == takerBid.amount),
            makerAsk.price,
            makerAsk.tokenId,
            makerAsk.amount,
            AssetType.ERC1155
        );
    }

    function canMatchMakerBid(Order calldata makerBid, Order calldata takerAsk)
        external
        pure
        override
        returns (
            bool,
            uint256,
            uint256,
            uint256,
            AssetType
        )
    {
        return (
            (makerBid.side != takerAsk.side) &&
            (makerBid.paymentToken == takerAsk.paymentToken) &&
            (makerBid.collection == takerAsk.collection) &&
            (makerBid.tokenId == takerAsk.tokenId) &&
            (makerBid.matchingPolicy == takerAsk.matchingPolicy) &&
            (makerBid.price == takerAsk.price) && 
            (makerBid.amount == takerAsk.amount),
            makerBid.price,
            makerBid.tokenId,
            makerBid.amount,
            AssetType.ERC1155
        );
    }
}