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

5 stars 0 forks source link

Short position owner will loose funds when put option expired #290

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#L494-L506

Vulnerability details

Impact

Fees are expected to be paid whenever an option is exercised (as per the function comment on L235).

However, the current protocol implementation also charges fees for expired put options. The owner of a short put option is subject to paying fees whenever the strike is returned for expired put options.

Proof of Concept

PuttyV2.withdraw

// 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); // @audit-info fee should not be paid if strike is simply returned to short owner for expired put options

    return;
}

Tools Used

Manual review

Recommended mitigation steps

Do not charge fees for expired put options by adapting the withdraw (L494-L506) implementation as following:

// 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;

    // @audit-info only charge fee if call is exercised
    if ((order.isCall && isExercised) && fee > 0) {
        feeAmount = (order.strike * fee) / 1000;
        ERC20(order.baseAsset).safeTransfer(owner(), feeAmount);
    }

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

    return;
}
ghost commented 2 years ago

Dup of https://github.com/code-423n4/2022-06-putty-findings/issues/380

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