search

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

5 stars 0 forks source link

QA Report #330

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Low

Putty position tokens can be used as underlying

Since the tokens Putty uses to represent positions are themselves ERC721s, it's possible to open and fill orders that use Putty position tokens as the underlying asset.

Here's an example order and fill on Rinkeby that use a Putty token as the underlying.

I don't see a clear exploit path here, but this is potentially unintentional behavior that could lead to unexpected results.

Recommendation: Check that the underlying asset in a Putty order is not a Putty position token.

Contract owner can frontrun withdrawals

The contract owner can observe and frontrun calls to withdraw to extract the maximum protocol fee.

Scenario:

  1. The fee parameter is set to 0.5%.
  2. The contract owner observes a call to withdraw in the mempool.
  3. The owner frontruns this transaction and calls setFee to set fees to the maximum value of 3%.
  4. The withdrawing account pays more in fees than expected at the time they sent their transaction.

Recommendation: This finding is already significantly mitigated by limiting the maximum fee and emitting an event. However, it's possible to further limit the owner's ability to make unexpected changes by ensuring the contract owner is a timelock proxy with a waiting period for parameter changes.

Options can be written with zero duration

It's possible for users to accidentally submit zero duration orders. If filled, the taker would collect the strike price, but the option would be immediately expired. Consider requiring both a minimum and maximum order duration.

Informational

Optimizer bug in Solidity 0.8.13 and 0.8.14

Solidity versions 0.8.13 and 0.8.14 are vulnerable to a recently reported optimizer bug related to inline assembly. Solidity 0.8.15 has been released with a fix.

This bug only occurs under very specific conditions: the legacy optimizer must be enabled rather than the IR pipeline (true for the current project configuration), and the affected assembly blocks must not refer to any local Solidity variables. The Solmate and OpenZeppelin libraries used in this project make use of inline assembly, but do not appear vulnerable. However, it's worth being aware of this vulnerability. Consider upgrading to Solidity 0.8.15.

Incorrect comments

outdoteth commented 2 years ago

Contract owner can frontrun withdrawals

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

Options can be written with zero duration

Duplicate: Orders with low durations can be easily DOS’d and prevent possibility of exercise: https://github.com/code-423n4/2022-06-putty-findings/issues/265

Optimizer bug in Solidity 0.8.13 and 0.8.14

Duplicate: Use of solidity 0.8.13 with known issues in ABI encoding and memory side effects: https://github.com/code-423n4/2022-06-putty-findings/issues/348