Omniscia Alliance Block Audit

LiquidityMiningCampaign Static Analysis Findings

LiquidityMiningCampaign Static Analysis Findings

LMC-01S: Inapplicacy of Checks-Effects-Interactions

Description:

The linked statements conduct external calls with a value initially retrieved from storage that is zeroed out after it has been utilized by the external call.

Example:

contracts/LiquidityMiningCampaign.sol
135uint256 finalRewards = user.tokensOwed[0].sub(userAccruedRewads[_userAddress]);
136uint256 userBonus;
137uint256 amountToStake;
138for (uint256 i = 0; i < lockSchemes.length; i++) {
139
140 uint256 additionalRewards = calculateProportionalRewards(_userAddress, finalRewards, lockSchemes[i]);
141 if(additionalRewards > 0) {
142 LockScheme(lockSchemes[i]).updateUserAccruedRewards(_userAddress, additionalRewards);
143 }
144 userBonus = LockScheme(lockSchemes[i]).exit(_userAddress);
145 amountToStake = amountToStake.add(userBonus);
146}
147
148amountToStake = amountToStake.add(user.tokensOwed[0]);
149_withdraw(user.amountStaked, _userAddress);
150user.tokensOwed[0] = 0;
151_claim(_userAddress);
152
153IERC20Detailed(rewardToken).safeApprove(_stakePool, amountToStake);
154StakeReceiver(_stakePool).delegateStake(_userAddress, amountToStake);
155userAccruedRewads[_userAddress] = 0;

Recommendation:

As external calls can lead to a re-entrancy, it is always best practice to apply the Checks-Effects-Interactions pattern. In the linked cases, the code should zero out the userAccruedRewads mapping directly after it has been utilized at the finalRewards calculation. The severity of this finding has been labelled as "minor" due to the _stakePool being whitelisted by an owner-based ACL, however, the pattern should be applied regardless to ensure security by design.

Alleviation:

The accrued rewards are properly zeroed out before any external transfers thus conforming to the Checks-Effects-Interactions pattern.