search

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

1 stars 0 forks source link

Missing events for critical parameter changing operations by owner #64

Open code423n4 opened 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, insurancePoolSwitchStage and whitelisting.

None of these setter functions emit events to record these changes on-chain for off-chain monitors/tools/interfaces to register the updates and react if necessary.

Impact: Malicious owner changes the critical addresses or values that significantly change the security posture/perception of the protocol. No events are emitted and users lose funds/confidence. The protocol takes a reputation hit.

Proof of Concept

See similar High-severity finding OpenZeppelin’s Audit of Audius (https://blog.openzeppelin.com/audius-contracts-audit/#high) and Medium-severity finding OpenZeppelin’s Audit of UMA Phase 4 (https://blog.openzeppelin.com/uma-audit-phase-4/)

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

Consider emitting events when these addresses/values are updated. This will be more transparent and it will make it easier to keep track of the status of the system.

raymogg commented 3 years ago

Duplicate of #66

cemozerr commented 3 years ago

Opening this issue as the event emission seems to be separate from the arbitrarily changing of the values.