Omniscia Alliance Block Audit
LiquidityMiningCampaign Static Analysis Findings
LiquidityMiningCampaign Static Analysis Findings
LMC-01S: Inapplicacy of Checks-Effects-Interactions
Type | Severity | Location |
---|---|---|
External Call Validation | Minor | LiquidityMiningCampaign.sol:L155 |
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:
135uint256 finalRewards = user.tokensOwed[0].sub(userAccruedRewads[_userAddress]);136uint256 userBonus;137uint256 amountToStake;138for (uint256 i = 0; i < lockSchemes.length; i++) {139140 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}147148amountToStake = amountToStake.add(user.tokensOwed[0]);149_withdraw(user.amountStaked, _userAddress);150user.tokensOwed[0] = 0;151_claim(_userAddress);152153IERC20Detailed(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.