Streaming Protocol contest - csanuragjain's results

Handle

csanuragjain

Vulnerability details

Impact

It was observed that any feePercent which is greater than MAX_FEE_PERCENT is already rejected in updateFeeParams function of StreamFactory. But on creating stream this is again checked in the constructor for feePercent<=100%

Proof of Concept

1. If user want to create new Stream with a fees he will first need to call updateFeeParams function of StreamFactory

function updateFeeParams(GovernableFeeParams memory newFeeParams) public governed {
        require(newFeeParams.feePercent <= MAX_FEE_PERCENT, "fee");
        GovernableFeeParams memory old = feeParams;
        feeParams = newFeeParams;
        emit FeeParametersUpdated(old, newFeeParams);
    }

2. As we can see this function checks that user provided fees does not cross max fees

3. After this user creates a new stream calling createStream function

4. This inturn calls the constructor of Stream contract

constructor(
        uint64 _streamId,
        address creator,
        bool _isSale,
        address _rewardToken,
        address _depositToken,
        uint32 _startTime,
        uint32 _streamDuration,
        uint32 _depositLockDuration,
        uint32 _rewardLockDuration,
        uint16 _feePercent,
        bool _feeEnabled

    )
        LockeERC20(_depositToken, _streamId, _startTime + _streamDuration)
        ExternallyGoverned(msg.sender) // inherit factory governance
        public 
    {

require(feePercent < 10000, "fee");
}

5. As we can see the contract is again checking for fees<=100% which is not required as fees coming to constructor will always be <=MAX_FEE_PERCENT

Recommended Mitigation Steps

Remove the below duplicate check imposed at constructor of Stream

require(feePercent < 10000, "fee");