Streaming Protocol contest - 0x1f8b's results

Handle

0x1f8b

Vulnerability details

Impact

Free flashloans for governance.

Proof of Concept

It's possible for governance to call a flashloan, pay the fee, and after it call claimFees, by this way, he always will have a free flashloan service.

Tools Used

Manual review

Recommended Mitigation Steps

Dificult to solve without distribute fees around the users

Handle

0x1f8b

Vulnerability details

Impact

Lack of inputs during deployment could produce a re-deploy with human errors.

Proof of Concept

The unique variable validated was feePercent, but all the address must be checked to be != address(0) and _startTime + _streamDuration must be less than the current block.

Source reference: https://github.com/code-423n4/2021-11-streaming/blob/56d81204a00fc949d29ddd277169690318b36821/Streaming/src/Locke.sol#L264-L274

Tools Used

Manual review

Recommended Mitigation Steps

Validate the input params, the address must be != address(0)

Handle

0x1f8b

Vulnerability details

Impact

Optimize code.

Proof of Concept

Name and symbol must be inmutable https://github.com/code-423n4/2021-11-streaming/blob/56d81204a00fc949d29ddd277169690318b36821/Streaming/src/LockeERC20.sol#L21-L23.

Tools Used

Manual review

Recommended Mitigation Steps

Set name and symbol as inmutable

Handle

0x1f8b

Vulnerability details

Impact

Gas optimization.

Proof of Concept

In the line https://github.com/code-423n4/2021-11-streaming/blob/56d81204a00fc949d29ddd277169690318b36821/Streaming/src/Locke.sol#L711 it was multiply by 10 and then divided by 10.000, it's better to do only a div by 1.000, because the result will be the same.

Tools Used

Manual review

Recommended Mitigation Steps

Change the line to: uint112 feeAmt = amount / 1000; // 10bps fee

Handle

0x1f8b

Vulnerability details

Impact

Lower gas consumition.

Proof of Concept

The variable decimals it's inmutable, but his value it's always the same, it's better to use const in order to avoid the storage access consumition. https://github.com/code-423n4/2021-11-streaming/blob/56d81204a00fc949d29ddd277169690318b36821/Streaming/src/LockeERC20.sol#L25

Tools Used

Manual review

Recommended Mitigation Steps

Use const for decimals

Handle

0x1f8b

Vulnerability details

Impact

Remove not used code is good for optimize the deployment and jumps.

Proof of Concept

state variable emergency_gov and externallyEmergencyGoverned and emergency_governed modifiers are not used, it's better to remove them from the code, or start to use this feature.

Tools Used

Manual review

Recommended Mitigation Steps

Remove dead code

Handle

0x1f8b

Vulnerability details

Impact

Used wrong type during construction.

Proof of Concept

In the method fundStream of Locke contract, we can se that three cast was done, in order to get the balance with balanceOf and do the transferFrom. This can be avoided if we store the variable as ERC20.

Tools Used

Manual review

Recommended Mitigation Steps

Store rewardToken as ERC20

Handle

0x1f8b

Vulnerability details

Impact

Empty blocks was found, it's good to remove them in order to spend less gas and improve the codding style.

Proof of Concept

The methods stake and withdraw has an empty block in the last condition, it's better to remove it in order to improve the codding style and gas expenses.

Source reference:

• https://github.com/code-423n4/2021-11-streaming/blob/56d81204a00fc949d29ddd277169690318b36821/Streaming/src/Locke.sol#L443
• https://github.com/code-423n4/2021-11-streaming/blob/56d81204a00fc949d29ddd277169690318b36821/Streaming/src/Locke.sol#L472

Tools Used

Manual review

Recommended Mitigation Steps

Remove empty blocks

Handle

0x1f8b

Vulnerability details

Impact

Gas optimization.

Proof of Concept

In the method exit of Locke contract, ts.tokens was stored in a local variable, amount, and then this variable was used for call withdraw method, is better to call directly like withdraw(ts.tokens)

Source reference:

• https://github.com/code-423n4/2021-11-streaming/blob/56d81204a00fc949d29ddd277169690318b36821/Streaming/src/Locke.sol#L492-L493

Tools Used

Manual review.

Recommended Mitigation Steps

Remove the amount variable.

Handle

0x1f8b

Vulnerability details

Impact

Unneded checks.

Proof of Concept

In the flashloan method of Stream contract there are a mistake in the last requires:

        if (token == depositToken) {
            depositTokenFlashloanFeeAmount += feeAmt;
            require(preDepositTokenBalance + feeAmt <= postDepositTokenBalance, "f1");
            require(preRewardTokenBalance <= postRewardTokenBalance, "f2");
        } else {
            rewardTokenFeeAmount += feeAmt;
            require(preDepositTokenBalance <= postDepositTokenBalance, "f3");
            require(preRewardTokenBalance + feeAmt <= postRewardTokenBalance, "f4");
        }

It check the balances from depositToken when it was rewardToken, and from rewardToken when it's depositToken, these checks are not required because only one transfer was done.

Source reference:

• https://github.com/code-423n4/2021-11-streaming/blob/56d81204a00fc949d29ddd277169690318b36821/Streaming/src/Locke.sol#L708-L721

Tools Used

Manual review

Recommended Mitigation Steps

Remove checks from the other token