Omniscia Alliance Block Audit
RewardsPoolBase Static Analysis Findings
RewardsPoolBase Static Analysis Findings
RPB-01S: Inapplicacy of Checks-Effects-Interactions Pattern
| Type | Severity | Location |
|---|---|---|
| External Call Validation | Minor | RewardsPoolBase.sol:L144-L148 |
Description:
The linked external call performs a transferFrom invocation on the staking token before updating the users reward debt.
Example:
144stakingToken.safeTransferFrom(145 address(chargeStaker ? staker : msg.sender),146 address(this),147 _tokenAmount148);149150for (uint256 i = 0; i < rewardsTokens.length; i++) {151 uint256 totalDebt =152 user.amountStaked.mul(accumulatedRewardMultiplier[i]).div(1e18); // Update user reward debt for each token153 user.rewardDebt[i] = totalDebt;154}155emit Staked(staker, _tokenAmount);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 case, the code should perform the external calls as the last statement of the code block. The severity of this finding has been labelled as "minor" due to the _rewardTokens being defined by the owner of the perspective factories, however, the pattern should be applied regardless to ensure security by design.
Alleviation:
The Checks-Effects-Interactions pattern was applied according to our suggestion by moving the transfer of tokens after any state adjustments.
RPB-02S: Inapplicacy of Checks-Effects-Interactions Pattern
| Type | Severity | Location |
|---|---|---|
| External Call Validation | Minor | RewardsPoolBase.sol:L196 |
Description:
The linked external call performs a transfer invocation on the staking token before updating the users reward debt.
Example:
196stakingToken.safeTransfer(address(withdrawer), _tokenAmount);197198for (uint256 i = 0; i < rewardsTokens.length; i++) {199 uint256 totalDebt =200 user.amountStaked.mul(accumulatedRewardMultiplier[i]).div(1e18); // Update user reward debt for each token201 user.rewardDebt[i] = totalDebt;202}203204emit Withdrawn(withdrawer, _tokenAmount);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 case, the code should perform the external calls as the last statement of the code block. The severity of this finding has been labelled as "minor" due to the _rewardTokens being defined by the owner of the perspective factories, however, the pattern should be applied regardless to ensure security by design.
Alleviation:
The Checks-Effects-Interactions pattern was applied by moving the outward transfer of tokens after any state changes.