Platform: Code4rena
Start Date: 30/11/2021
Pot Size: $100,000 USDC
Total HM: 15
Participants: 36
Period: 7 days
Judge: 0xean
Total Solo HM: 4
Id: 62
League: ETH
Rank: 9/36
Findings: 4
Award: $3,844.48
🌟 Selected for report: 7
🚀 Solo Findings: 1
0x0x0x
On recoverTokens function in Stream. Excess amount of deposit token is calculated as follows:
uint256 excess = ERC20(token).balanceOf(address(this)) - (depositTokenAmount - redeemedDepositTokens);
This calculation does not include depositTokenFlashloanFeeAmount. Therefore they can be claimed by the streamCreator altough they are for factory reward. I consider this as a high risk, since profits of factory can get stolen and anyone create a stream.
Futhermore, those fees can be still claimed by the governance, which results at less than expected depositToken in contract. Therefore, user funds get lost.
Add depositTokenFlashloanFeeAmount to the calculation.
#0 - 0xean
2022-01-14T20:51:14Z
duplicate of #241
0x0x0x
Creating a stream, where depositToken == rewardToken might be a use case. But since amounts of both of them are accumulated in different variables and there is a recoverTokens function. When depositToken == rewardToken, one can easily organize scams using stream protocol. Easiest example is that stream creator can withdraw all rewards whenever possible. Furthermore, many functionalities of the contract don't work properly.
Add to constructor of stream constructor a requirement to make sure depositToken != rewardToken.
#0 - 0xean
2022-01-14T21:01:45Z
dupe of #215
🌟 Selected for report: 0x0x0x
2417.4456 USDC - $2,417.45
0x0x0x
Some fee on transfer tokens, do not reduce the fee directly from the transferred amount, but subtracts it from remaining balance of sender. Some tokens prefer this approach, to make the amount received by the recipient an exact amount. Therefore, after funds are send to users, balance becomes less than it should be. So this contract does not fully support fee on transfer tokens. With such tokens, user funds can get lost after transfers.
I don't recommend directly claiming to support fee on transfer tokens. Current contract only supports them, if they reduce the fee from the transfer amount.
#0 - brockelmore
2021-12-08T22:00:19Z
We will make this clear for stream creators
🌟 Selected for report: ye0lde
Also found by: 0x0x0x, Jujic, pedroais, pmerkleplant
13.1496 USDC - $13.15
0x0x0x
!= 0 is a cheaper operation compared to > 0, when dealing with uint.
./Locke.sol:226: if (acctTimeDelta > 0 && ts.tokens > 0) { ./Locke.sol:236: if (tdelta > 0 && unstreamed > 0) { ./Locke.sol:378: require(amount > 0, "amt"); ./Locke.sol:418: require(amount > 0, "amt"); ./Locke.sol:456: require(amount > 0, "amt"); ./Locke.sol:522: require(amount > 0, "amt"); ./Locke.sol:536: require(amount > 0, "amt"); ./Locke.sol:572: require(rewardAmt > 0, "amt"); ./Locke.sol:612: if (fees > 0) { ./Locke.sol:621: if (fees > 0) { ./Locke.sol:679: if (incentives[token] > 0) {
#0 - 0xean
2022-01-16T14:20:30Z
dupe of #143
🌟 Selected for report: 0x0x0x
Also found by: csanuragjain
0x0x0x
require(feePercent < 10000, "fee");
Since we already make sure that this is less than 50 bps in factory. There is no need to do this check.
🌟 Selected for report: 0x0x0x
0x0x0x
amount = uint112(newBal - prevBal); // if fee is enabled, take a fee if (feeEnabled) { // Safety: // 1. feePercent & y are casted up to u256, so cannot overflow when multiplying // 2. downcast is safe because (x*y)/MAX_X is guaranteed to be smaller than y which is uint112 // 3. amount is guaranteed to be greater than feeAmt uint112 feeAmt; unchecked { feeAmt = uint112(uint256(feePercent) * uint256(amount) / 10000); amt = amount - feeAmt; } // since this operation can be repeated, we cannot assume no overflow so use checked math rewardTokenFeeAmount += feeAmt; rewardTokenAmount += amt; } else { amt = amount; rewardTokenAmount += amt; }
This code block can be implemented more efficiently. In else part there is no reason to cache amount at amt and then add to rewardTokenAmount, since one can directly add amt to rewardTokenAmount. Furthermore, then amt should be declared inside if statement, since otherwi
0x0x0x
When exit is called updateStream is called first and after that it is called, when withdraw is executed. This significantly increases gas cost and can be avoided by introducing an internal _withdraw function containing the required operations.
0x0x0x
uint112 feeAmt = amount * 10 / 10000; // 10bps fee
One can simply replace it with
uint112 feeAmt = amount / 1000; // 10bps fee
and save gas without sacrificing any precision.
🌟 Selected for report: 0x0x0x
0x0x0x
In claimReward, lastApplicableTime is called to update lastUpdate, but stream did already end so calling this function wastes gas. One can directly assign endStream as lastUpdate.
🌟 Selected for report: 0x0x0x
0x0x0x
Current function containts this code block.
ts.rewards = earned(ts, cumulativeRewardPerToken); // update users last cumulative reward per token ts.lastCumulativeRewardPerToken = cumulativeRewardPerToken; lastUpdate = lastApplicableTime(); uint256 rewardAmt = ts.rewards; ts.rewards = 0; require(rewardAmt > 0, "amt");
The following block consumes less gas and computes the same result
uint256 rewardAmt = earned(ts, cumulativeRewardPerToken); // update users last cumulative reward per token ts.lastCumulativeRewardPerToken = cumulativeRewardPerToken; lastUpdate = lastApplicableTime(); ts.rewards = 0; require(rewardAmt > 0, "amt");
By directly saving earned in rewardAmt less operations are required and ts.rewards are set to 0 at the end of the function, so it provides the same logic
45.0947 USDC - $45.09
0x0x0x
There is no need to cache b1 and temp. It can be avoided to save gas.
#0 - 0xean
2022-01-20T17:47:54Z
dupe of #239