Lack of security Mechanism.
In the comment it says:
comment "Using >= instead of == for additional safety reasons. In the event of an unforeseen increase in the withdrawn amount, the stream will still be marked as depleted"
So there is a chance the amounts.withdrawn will be more than amounts.deposited due to a hack for example.
The developers considered the possibility that amounts.withdrawn would be greater than amounts.deposited, but did not consider the possibility
that the excess amount was coming from amount parameter because the only place where amounts.withdrawn is increased is in _withdraw
,
which is done through _streams[streamId].amounts.withdrawn = _streams[streamId].amounts.withdrawn + amount;
The amount is the reason for the increase in amounts.withdrawn, and in this case funds will be taken from other users, because amount passed
directly into transferFrom without checking amount value in case of amounts.withdrawn > amounts.deposited
and this will cause the loss of users’ funds.
Lets take this scenario:
funder deposited 1000 DAI to recipient X so the amounts.deposited is 1000 DAI.
The last withdraw: amounts.withdrawn = amounts.withdrawn + amount
Eg: amounts.withdrawn(900) + amount(300) -> 1200 now the withdrawn is more than deposited amount.
then if statement is executed and set isDepleted.
now recipient will get 200 extra tokens, these 200 tokens is taken from the contract which is another recipient funds.
If the last recipient deposited 200 his funds will be lost.
NOTE:
since if statement execute only at the last withdrawn the amounts.withdrawn will be ">=" when we add amount
because amounts.withdrawn increase only in _withdraw
so for amounts.withdrawn to be ">=" amount should be more
than it should be in this case if (amounts.withdrawn > amounts.deposited - amounts.refunded) -> true
and if there is no mechanism to correct the amount it will send tokens to recipient more than it should be.
Amount of tokens could be 1000 as it could be millions.
And since the protocol is non-upgradeable such mistake could make the protocol lose millions.
Lack of security mechanism could lead protocol to lose millions better safe than sorry.
Manual
Since if statement execute only at the last withdraw add a Mechanism to send to the recipient only to proper amount of tokens, add this inside if statement.
The contest is live. Earn rewards by submitting a finding.
This is your time to appeal against judgements on your submissions.
Appeals are being carefully reviewed by our judges.