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

5 stars 0 forks source link

QA Report #429

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

[L-01] Can cancel the same order multiple times.

Effects: In case it was already filled, this operation:

  1. Emit a misleading CancelledOrder event,
  2. Not informative to maker as his call to cancel was not reverted.

Mitigation:

function cancel(Order memory order) public {
    require(msg.sender == order.maker, "Not your order");

    bytes32 orderHash = hashOrder(order);

    require(!cancelledOrders[orderHash], "Order already cancelled");  //@audit add this line here

    // mark the order as cancelled
    cancelledOrders[orderHash] = true;

    emit CancelledOrder(orderHash, order);
}

[N-01] Ambiguous error message in batchFillOrder:

        require(orders.length == signatures.length, "Length mismatch in input");
        require(signatures.length == floorAssetTokenIds.length, "Length mismatch in input");

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

Can specify which two mismatch to help user re-send correct transaction.

[N-02] Ambiguous error message in constructor:

require(_weth != address(0), "Unset weth address"); https://github.com/code-423n4/2022-06-putty/blob/3b6b844bc39e897bd0bbb69897f2deff12dc3893/contracts/src/PuttyV2.sol#L214

Better to say "Invalid weth address" or "weth address cannot be 0"

[N-03] Inconsistent format of error messages

All error messages on PuttyV2Nft.sol are ALL_CAPICATAL and snake cased, while all error messages on PuttyV2.sol are natural English language (only first letter is capital and use spaces) "INVALID_RECIPIENT" in compare to "Length mismatch in input"

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