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.

Proof of Concept

devAddr: https://github.com/sushiswap/miso/blob/2cdb1486a55ded55c81898b7be8811cb68cfda9e/contracts/Access/MISOAccessFactory.sol#L22 https://github.com/sushiswap/miso/blob/2cdb1486a55ded55c81898b7be8811cb68cfda9e/contracts/Access/MISOAccessFactory.sol#L85 https://github.com/sushiswap/miso/blob/2cdb1486a55ded55c81898b7be8811cb68cfda9e/contracts/Access/MISOAccessFactory.sol#L109-L116

misoDiv: https://github.com/sushiswap/miso/blob/2cdb1486a55ded55c81898b7be8811cb68cfda9e/contracts/Access/ListFactory.sol#L73 https://github.com/sushiswap/miso/blob/2cdb1486a55ded55c81898b7be8811cb68cfda9e/contracts/Access/ListFactory.sol#L170 https://github.com/sushiswap/miso/blob/2cdb1486a55ded55c81898b7be8811cb68cfda9e/contracts/Access/ListFactory.sol#L137-L140

bonusEndBlock & bonusEndMultiplier: https://github.com/sushiswap/miso/blob/2cdb1486a55ded55c81898b7be8811cb68cfda9e/contracts/Farms/MISOMasterChef.sol#L82-L86 https://github.com/sushiswap/miso/blob/2cdb1486a55ded55c81898b7be8811cb68cfda9e/contracts/Farms/MISOMasterChef.sol#L218-L225 https://github.com/sushiswap/miso/blob/2cdb1486a55ded55c81898b7be8811cb68cfda9e/contracts/Farms/MISOMasterChef.sol#L160-L171

devPercentage: https://github.com/sushiswap/miso/blob/2cdb1486a55ded55c81898b7be8811cb68cfda9e/contracts/Farms/MISOMasterChef.sol#L76 https://github.com/sushiswap/miso/blob/2cdb1486a55ded55c81898b7be8811cb68cfda9e/contracts/Farms/MISOMasterChef.sol#L269-L271 https://github.com/sushiswap/miso/blob/2cdb1486a55ded55c81898b7be8811cb68cfda9e/contracts/Farms/MISOMasterChef.sol#L353-L355 https://github.com/sushiswap/miso/blob/2cdb1486a55ded55c81898b7be8811cb68cfda9e/contracts/Farms/MISOMasterChef.sol#L378-L382

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.

[Clearwood]

As additional verification can be executed on the UI level on whether these parameters have been set, I do not believe this to be necessary

[ghoul-sol]

This is best practice recommendation