Missing events for critical parameter changing operations by owner

[code423n4]

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]

Duplicate of #66

[cemozerr]

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