Streaming Protocol contest - mtz's results

Handle

mtz

Vulnerability details

Impact

The constraint (mentioned in the documentation) that feePercent is less than 5% is only validated in StreamFactory not in Stream. In Stream it is only checked that feePercent is less than 100% see here. However, Stream can be created without a StreamFactory. This can lead a user/funder to falsely assume the fee is lower than 5%. This is particularly true given that the feePercent is expressed as a fraction of 10000. For example, a user/funder can call feeParams which returns (true, 5000), indicating a fee of 50%. After reading the documentation, the user /funder or a staker might assume that 5000 means 5% instead of 50% since 50% is not possible according to the documentation.

Proof of Concept

Stream stream = new Stream(1, msg.sender, false, rewardToken, depositTOken, startTime, streamDuration, depositLockDuration, rewardLockDuration, 
5000, // the important part
true);

Tools Used

None

Recommended Mitigation Steps

Change this check to the following: require(feePercent < 500 "fee");

Handle

mtz

Vulnerability details

Impact

The merkleAccess field on the TokenStream struct (source code here) is unused. This will result in unnecessary gas costs whenever a TokenStream is accessed or modified (such as during every updateStream call). Because TokenStreams are stored in storage, this extra cost will be particularly expensive. This field takes up a new storage slot, since each slot is 256 bits: lastCumulativeRewardPerToken takes up 1 slot. virtualBalance takes up 1 slot. rewards, tokens and lastUpdate take up 1 slot. merkleAccess takes up another slot.

Proof of Concept

N/A

Tools Used

N/A

Recommended Mitigation Steps

Delete merkleAccess from TokenStream.

Handle

mtz

Vulnerability details

Impact

The solidity version pragma used in Locke.sol allows Locke.sol to be compiled with any 0.8.X version of solidity. It is better to pick a specific version of solidity to avoid a scenario where one version of solidity is used to test the code locally and another version is used to deploy the contract. This issue is even worse in LockeERC20.sol where the pragma allows the contract to be compiled using any version of solidity greater than 0.8.0. This is particularly bad since: 1. It is inconsistent with Locke.sol 2. LockeERC20.sol can be compiled with a version of solidity that has different semantics than the version LockeERC20.sol was developed with, potentially leading to security issues.

Proof of Concept

N/A

Tools Used

None

Recommended Mitigation Steps

Specify an exact version of solidity that is used. This is done by the original Uniswap code for example.