code-423n4 / 2021-04-vader-findings

1 stars 0 forks source link

Missing event for critical changeUTILS() function in Vader.sol #241

Closed code423n4 closed 3 years ago

code423n4 commented 3 years ago

Handle

0xRajeev

Vulnerability details

Impact

The changeUTILS() function changes the address for the Utils contract which provides all the critical functions in the protocol for math calculations, IL, lending, incentives, pricing and system operations. However, this function is missing an event emission for off-chain monitoring tools to monitor this critical on-chain change.

Proof of Concept

https://github.com/code-423n4/2021-04-vader/blob/3041f20c920821b89d01f652867d5207d18c8703/vader-protocol/contracts/Vader.sol#L188-L191

Tools Used

Manual Analysis

Recommended Mitigation Steps

Add a changeUTILS event and emit that at the end of changeUTILS() function.

strictly-scarce commented 3 years ago

In order to change the Utils via DAO, a lot of events are emitted on the DAO side, so disagree with severity.

strictly-scarce commented 3 years ago

But perhaps a specific event can be added, purely for ease of monitoring.

Mervyn853 commented 3 years ago

Our decision matrix for severity:

0: No-risk: Code style, clarity, off-chain monitoring (events etc), exclude gas-optimisations 1: Low Risk: UX, state handling, function incorrect as to spec 2: Funds-Not-At-Risk, but can impact the functioning of the protocol, or leak value with a hypothetical attack path with stated assumptions, but external requirements 3: Funds can be stolen/lost directly, or indirectly if a valid attack path shown that does not have handwavey hypotheticals.

Recommended: 1

dmvt commented 3 years ago

duplicate of #250