search

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

5 stars 0 forks source link

QA Report #378

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

PuttyV2 QA Report

Summary / Impression

Risk Title
L01 Cancel the same order will emit event again
N01 Usage of magic number
OOS01 Mismatch to the spec file
OOS02 The frontend error for minus values

Low

L01: Cancel the same order will emit event again

github link

One can cancel the same order and emit CancelledOrder again.

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

        bytes32 orderHash = hashOrder(order);

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

        emit CancelledOrder(orderHash, order);
    }

Non critical

N01: Usage of magic number

github link

The precision of the fee, 1000, can be stored as a constant for easy referring.

 feeAmount = (order.strike * fee) / 1000;

Out Of Scope

OOS01: Mismatch to the spec file

github link

// spec/withdraw.md

    input                                        │ *check position side is equal to short     │
    ┌─────────────────────────────┐              │                                            │
    │ order: Order                ├──────────────► *check position has expired OR it has been │
    │ floorAssetTokeIds: number[] │              │  exercised                                 │
    └─────────────────────────────┘              │                                            │

The spec for withdraw shows that the function takes two inputs: order and floorAssetTokeIds. But the implementation only takes order as an input.

 function withdraw(Order memory order) public {

OOS02: The frontend error for minus values

link

In the create order, the asset type NFT will allow to go to minus id. The asset type ERC20 will allow to go to minus values. The asset type Floor will error out when the token amount goes to minus. Disclaimer: Using firefox.