hickuphh3 - Insufficient input validation of escrowPortion and escrowPool

[sherlock-admin]

hickuphh3

medium

Insufficient input validation of escrowPortion and escrowPool

Summary

There could be a loss of rewards if escrowPool and escrowPortion are misconfigured.

Vulnerability Detail

escrowPortion determines how much of the rewards should be escrowed to the escrowPool. Note that it is possible for the escrowPool to be set to the null address. Thus, the escrowPortion and escrowPool should be checked to be tightly coupled. Specifically, the following condition should hold true: escrowPortion != 0 => escrowPool != address(0)

There is otherwise a loss of rewards: if escrowPortion is non-zero, but the pool is set to the null address, escrowPool.deposit() will not be called, and the escrowed portion will be unrecoverable from the contract.

Impact

Rewards are lost if escrowPortion is non-zero but the escrow pool address is the zero address.

Code Snippet

https://github.com/Merit-Circle/merit-liquidity-mining/blob/ce5feaae19126079d309ac8dd9a81372648437f1/contracts/base/BasePool.sol#L102-L107

Tool used

Manual Review

Recommendation

In the constructor, ensure that the escrow pool is non-zero if the escrow portion is non-zero.

if (_escrowPortion != 0 && _escrowPool == address(0)) revert EscrowSettingError();

Note that if _escrowPortion is zero then it doesn't really matter if _escrowPool is non-zero; no rewards will be escrowed.

Duplicate of #107