code-423n4 / 2021-06-tracer-findings

1 stars 0 forks source link

Missing timelock for critical parameter changing operations by owner
 #65

Closed code423n4 closed 3 years ago

code423n4 commented 3 years ago

Handle

0xRajeev

Vulnerability details

Impact

The owner of TracerPerpetualSwaps contract, who is potentially untrusted as per specification, can change the market critical parameters such as the addresses of the Liquidation/Pricing/Insurance/GasOracle/FeeReceiver and also critical values such as feeRate, maxLeverage, fundingRateSensitivity, deleveragingCliff, lowestMaxLeverage and insurancePoolSwitchStage.

None of these changes provide a window of opportunity for the market participants to react and adjust their participation accordingly i.e. continue to engage or withdraw if changes are deemed risky or unprofitable.

Impact: Malicious owner changes the critical addresses or values that significantly change the security posture/perception and/or profitability of the protocol. Users cannot disengage in time and lose profits/funds/confidence. The protocol takes a reputation hit.

Proof of Concept

See similar Medium-severity finding in ConsenSys's Audit of 1inch Liquidity Protocol (https://consensys.net/diligence/audits/2020/12/1inch-liquidity-protocol/#unpredictable-behavior-for-users-due-to-admin-front-running-or-general-bad-timing)

https://github.com/code-423n4/2021-06-tracer/blob/74e720ee100fd027c592ea44f272231ad4dfa2ab/src/contracts/TracerPerpetualSwaps.sol#L522-L570

https://github.com/code-423n4/2021-06-tracer/blob/74e720ee100fd027c592ea44f272231ad4dfa2ab/src/contracts/TracerPerpetualSwaps.sol#L577-L585

Tools Used

Manual Analysis

Recommended Mitigation Steps

Consensys’s Recommendation for the above referenced similar finding: “ We recommend giving the user advance notice of changes with a time lock. For example, make all system-parameter and upgrades require two steps with a mandatory time window between them. The first step merely broadcasts to users that a particular change is coming, and the second step commits that change after a suitable waiting period. This allows users that do not accept the change to withdraw immediately.”

raymogg commented 3 years ago

Duplicate of #66