Treasury Static Analysis Findings

TRE-01S: Inapplicacy of Checks-Effects-Interactions Pattern

Description:

The linked statements conduct external calls with values retrieved from storage that are zeroed out after they have been utilized by the external call.

Example:

Copy
38function returnLiquidity(address[] calldata rewardPools, uint[] calldata externalRewards) public onlyOwner {
39    require(rewardPools.length == externalRewards.length, "returnLiquidity:: pools and external tokens do not match");
40    for (uint256 i = 0; i < rewardPools.length; i++) {
41        address stakingToken = IRewardsPoolBase(rewardPools[i]).stakingToken();
42        IERC20Detailed(stakingToken).safeTransfer(rewardPools[i], liquidityDrawn[rewardPools[i]]);
43        liquidityDrawn[rewardPools[i]] = 0;
44        if(externalRewards[i] == 0) {
45            continue;
46        }
47        IERC20Detailed(externalRewardToken).safeApprove(rewardPools[i], externalRewards[i]);
48        ITreasuryOperated(rewardPools[i]).notifyExternalReward(externalRewards[i]);
49    }
50}

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 store the variables to a temporary in-memory slot, zero them out and then perform the external call they are meant to accompany. The severity of this finding has been labelled as "minor" due to the rewardPools being defined by the owner-caller of the function, however, the pattern should be applied regardless to ensure security by design.

Alleviation:

The Checks-Effects-Interactions pattern was applied by zeroing out the liquidity that is drawn before utilizing it in the external function call.