deployMarket may revert due to integer underflow from missing threshold check

[code423n4]

Handle

0xRajeev

Vulnerability details

Impact

If integratorFeePct is not reset via setter (where 1000 upper threshold is checked) but set only at init where the deployer assumes traditional basis points notation (100 basis points = 1%) and sets integratorFeePct to >1000 (assuming >10% but actually >100% in contract’s notation), e.g. 2000 for 20%, then integratorFee here will be > misoFee causing the subtraction in next line to underflow (because of lack of SafeMath and solc < 0.8.x) and setting a very high value for misoFee. This will cause the transfer on L256 to revert because the provided ETH will be less than the expected misoFee and therefore deployMarket will fail causing a DoS.

Proof of Concept

https://github.com/sushiswap/miso/blob/2cdb1486a55ded55c81898b7be8811cb68cfda9e/contracts/MISOFarmFactory.sol#L85

https://github.com/sushiswap/miso/blob/2cdb1486a55ded55c81898b7be8811cb68cfda9e/contracts/MISOFarmFactory.sol#L125

https://github.com/sushiswap/miso/blob/2cdb1486a55ded55c81898b7be8811cb68cfda9e/contracts/MISOFarmFactory.sol#L146-L157

https://github.com/sushiswap/miso/blob/2cdb1486a55ded55c81898b7be8811cb68cfda9e/contracts/MISOFarmFactory.sol#L236-L237

https://github.com/sushiswap/miso/blob/2cdb1486a55ded55c81898b7be8811cb68cfda9e/contracts/MISOFarmFactory.sol#L243-L245

Tools Used

Manual Analysis

Recommended Mitigation Steps

1. Add threshold check <= 1000 to integratorFeePct in initMISOFarmFactory()
2. Use BoringMath .sub or solc >= 0.8.0
3. Use standard basis points notation where 100 (not 10) basis points = 1%

[Clearwood]

As this parameter is under Sushi control and will likely remain at 0, this is a non issue

[ghoul-sol]

A valid concern worth low risk.