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

5 stars 0 forks source link

The fee is not paid as intended for put orders #376

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

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

Vulnerability details

Impact

MEDIUM - functions of the protocol could be impacted

For put options, the fees are not paid as intended.

Proof of Concept

As this comment says, fee is applied on exercise.

        @notice Fee rate that is applied on exercise.

But the current implementation applies the fee in withdraw:

    // withdraw function, where the fee is applied  

        // transfer strike to owner if put is expired or call is exercised
        if ((order.isCall && isExercised) || (!order.isCall && !isExercised)) {
            // send the fee to the admin/DAO if fee is greater than 0%
            uint256 feeAmount = 0;
            if (fee > 0) {
                feeAmount = (order.strike * fee) / 1000;
                ERC20(order.baseAsset).safeTransfer(owner(), feeAmount);
            }

            ERC20(order.baseAsset).safeTransfer(msg.sender, order.strike - feeAmount);

            return;
        }

Tools Used

foundry

Recommended Mitigation Steps

The fees can be applied in the exercise.

outdoteth commented 2 years ago

Duplicate: Fees are only applied on puts if they are expired: https://github.com/code-423n4/2022-06-putty-findings/issues/269