code-423n4 / 2022-05-cally-findings

2 stars 0 forks source link

QA Report #294

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Low

[L-01] Owner can frontrun exercise to increase fees

A malicious owner account can observe and frontrun calls to exercise and extract 100% of the strike price as a protocol fee.

Scenario:

  1. A malicious owner observes a call to exercise in the mempool.
  2. The owner frontruns exercise and calls setFee to set feeRate to 100%
  3. The full strike price is paid as a protocol fee, and 0 ETH are credited to the vault beneficiary.

Recommendation: Ensure the contract owner is a timelock proxy with a waiting period for parameter changes. Emit an event on changes to feeRate (See N-01 below).

[L-02] Beneficiary is credited additional ETH above premium

The Cally#buyOption function ensures that the caller sends an ETH amount equal to or greater than the calculated premium:

buyOption#L224

       require(msg.value >= premium, "Incorrect ETH amount sent");

It then credits the beneficiary with an amount equal to msg.value:

buyOption#L250

        ethBalance[beneficiary] += msg.value;

If the caller of buyOption sends excess ETH above the premium amount, this additional amount is credited to the beneficiary.

Recommendation: If this is intentional, clearly document this behavior for end users. If not, consider requiring an exact premium amount rather than accepting additional ETH.

QA

[N-01] Emit events for permissioned parameter changes

The permissioned function setFee updates the feeRate parameter, but does not emit an event. Consider emitting a SetFee event that logs the previous and new feeRate values. This enables you to monitor off chain for suspicious activity, and allows end users to observe and trust permissioned changes to this parameter.

Cally.sol#setFee

    /// @notice Sets the fee that is applied on exercise
    /// @param feeRate_ The new fee rate: fee = 1% = (1 / 100) * 1e18
    function setFee(uint256 feeRate_) external onlyOwner {
        feeRate = feeRate_;
    }
outdoteth commented 2 years ago

these can be bumped to medium severity: [L-01] Owner can frontrun exercise to increase fees: https://github.com/code-423n4/2022-05-cally-findings/issues/47 [L-02] Beneficiary is credited additional ETH above premium: https://github.com/code-423n4/2022-05-cally-findings/issues/84

HardlyDifficult commented 2 years ago

Moved L-01 to https://github.com/code-423n4/2022-05-cally-findings/issues/323

HardlyDifficult commented 2 years ago

Moved L-02 to https://github.com/code-423n4/2022-05-cally-findings/issues/324