Protocol fee rate can be arbitrarily modified by the admin/DAO and will apply to all existing options

[code423n4]

Lines of code

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

Vulnerability details

Impact

The admin/DAO can set a fee rate that is applied whenever an option is exercised (as per the function comment on L235).

A taker filling a call option knows the current protocol fee in advance and can estimate the receivable strike (strike - fee) at the time being. However, the fee is subject to change anytime, hence the taker could receive less at the time of withdrawing the strike as initially (at the time of filling the option) anticipated.

Proof of Concept

PuttyV2.setFee

/**
    @notice Sets a new fee rate that is applied on exercise. The
            fee has a precision of 1 decimal. e.g. 1000 = 100%,
            100 = 10%, 1 = 0.1%. Admin/DAO only.
    @param _fee The new fee rate to use.
    */
function setFee(uint256 _fee) public payable onlyOwner {
    require(_fee < 30, "fee must be less than 3%");

    fee = _fee;

    emit NewFee(_fee);
}

PuttyV2.withdraw

if (fee > 0) {
    feeAmount = (order.strike * fee) / 1000; // @audit-info current protocol fee is used. Fee can be different than at the time of the order being filled
    ERC20(order.baseAsset).safeTransfer(owner(), feeAmount);
}

Tools Used

Manual review

Recommended mitigation steps

1. Consider making the protocol fee rate a constant (once set it can not be changed)
2. Changes to the protocol fee only apply to options filled after the rate is updated (by storing the current fee rate per order hash within the fillOrder function)

[outdoteth]

Duplicate: Admin can change fee at any time for existing orders: https://github.com/code-423n4/2022-06-putty-findings/issues/422