Relying on setters for initialisation of critical parameters is risky

[code423n4]

Handle

0xRajeev

Vulnerability details

Impact

Critical contract parameters should have a fail-safe default value enforced during declaration or in constructor/initialize functions without relying on admin-controlled setters to be called. There are many such parameters across contracts here. Missed timely calling of such setters may result in type-dependent default values which could be dangerous for 1) address parameters where the default 0 address may result in reverts or worse token/ETH burn and 2) uint fees/dividends/etc. whose default value may be unexpected for the protocol.

insuranceFee and insurancePoolFee are never initialized in the constructor and depend on setters to be called by Governance.

Proof of Concept

insuranceFee & insurancePoolFee: https://github.com/code-423n4/2021-09-yaxis/blob/cf7d9448e70b5c1163a1773adb4709d9d6ad6c99/contracts/v3/Manager.sol#L43-L44 https://github.com/code-423n4/2021-09-yaxis/blob/cf7d9448e70b5c1163a1773adb4709d9d6ad6c99/contracts/v3/Manager.sol#L258-L267 https://github.com/code-423n4/2021-09-yaxis/blob/cf7d9448e70b5c1163a1773adb4709d9d6ad6c99/contracts/v3/Manager.sol#L283-L297

Tools Used

Manual Analysis

Recommended Mitigation Steps

Initialize all contract state variables with fail-safe default values at declaration or in constructor/initialize functions without depending on the timely calling of their setters. Enforce appropriate sanity/threshold checks at all such initializations.

[uN2RVw5q]

I partly agree with the "sponsor disputed" tag. This could also be changed to a documentation issue.

[GalloDaSballo]

Disagree with finding, default value is 0, which is meaningful (they take no fee)