Blur Exchange contest - rvierdiiev'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#L433

Vulnerability details

Impact

Order amount value is ignored. If seller created order to sell amount of 3 ERC1155 tokens and buyer execute that order then in the end buyer will pay all price amiunt, but will receive only 1 token.

BlurExchange.execute function receives token amount that should be sent to buyer from matching policies.

function _canMatchOrders(Order calldata sell, Order calldata buy)
        internal
        view
        returns (uint256 price, uint256 tokenId, uint256 amount, AssetType assetType)
    {
        bool canMatch;
        if (sell.listingTime <= buy.listingTime) {
            /* Seller is maker. */
            require(policyManager.isPolicyWhitelisted(sell.matchingPolicy), "Policy is not whitelisted");
            (canMatch, price, tokenId, amount, assetType) = IMatchingPolicy(sell.matchingPolicy).canMatchMakerAsk(sell, buy);
        } else {
            /* Buyer is maker. */
            require(policyManager.isPolicyWhitelisted(buy.matchingPolicy), "Policy is not whitelisted");
            (canMatch, price, tokenId, amount, assetType) = IMatchingPolicy(buy.matchingPolicy).canMatchMakerBid(buy, sell);
        }
        require(canMatch, "Orders cannot be matched");

        return (price, tokenId, amount, assetType);
    }

However StandardPolicyERC1155 always returns the amount of 1 and do not check the amount that was provided by seller and buyer. 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 means that buyer will pay more if amount of seller order is bigger then 1. And the main thing is that he paid for few tokens, but received only 1.

Proof of Concept

Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept.

Tools Used

Recommended Mitigation Steps

If provided amount is 0 then do not send tokens. Add checking of the identical amount in seller's order and buyer's order. And then return correct amount.

1. For the loops use ++i instead of i++. Also cache array size into variable. Use uint256 instead of any other uint types. And do not initialize I value with 0. https://github.com/code-423n4/2022-10-blur/blob/main/contracts/BlurExchange.sol#L199-L201 https://github.com/code-423n4/2022-10-blur/blob/main/contracts/BlurExchange.sol#L476 https://github.com/code-423n4/2022-10-blur/blob/main/contracts/PolicyManager.sol#L77 https://github.com/code-423n4/2022-10-blur/blob/main/contracts/lib/EIP712.sol#L77 https://github.com/code-423n4/2022-10-blur/blob/main/contracts/lib/MerkleVerifier.sol#L38
2. Use uint256 value 1 and 2 instead of boolean to save gas. https://github.com/code-423n4/2022-10-blur/blob/main/contracts/lib/ReentrancyGuarded.sol#L10