search

sherlock-audit / 2024-01-telcoin-judging

6 stars 5 forks source link

Tricko - Funds can be lost when changing stream parameters in `CouncilMember` contract. #112

Closed sherlock-admin closed 7 months ago

sherlock-admin commented 7 months ago

Tricko

high

Funds can be lost when changing stream parameters in CouncilMember contract.

Summary

Because CouncilMember._retrieve() isn't called prior to modifying stream parameters via either CouncilMember.updateID() or CouncilMember.updateTarget(), any funds accrued since the last _retrieve() call will be forfeited.

Vulnerability Detail

For accurate token distribution, it's crucial to call the internal method CouncilMember._retrieve() before significant state changes in the contract, such as token minting or claims. Unfortunately, this step is omitted before important methods like CouncilMember.updateTarget() or CouncilMember.updateID(). Consequently, funds accrued in the stream but not yet withdrawn won't be distributed.

For example, if the stream is switched, necessitating a modification of the _id parameter via CouncilMember.updateID(), all funds accumulated in the previous stream since the last _retrieve() call will become inaccessible.

Impact

Loss of funds during stream migrations.

Code Snippet

https://github.com/sherlock-audit/2024-01-telcoin/blob/0954297f4fefac82d45a79c73f3a4b8eb25f10e9/telcoin-audit/contracts/sablier/core/CouncilMember.sol#L236-L256

Tool used

Manual Review

Recommendation

Consider calling CouncilMember._retrieve() before setting the new values of _target and _id, so that funds distribution is done before changing stream parameters.

Duplicate of #99

sherlock-admin2 commented 7 months ago

1 comment(s) were left on this issue during the judging contest.

takarez commented:

invalid because { The function in question are the updateID and there's a governance modifier in place; making it invalid according to sherlock rule}

nevillehuang commented 7 months ago

request poc

sherlock-admin2 commented 7 months ago

PoC requested from @retsoko

Requests remaining: 9

amshirif commented 7 months ago

Duplicate issue https://github.com/sherlock-audit/2024-01-telcoin-judging/issues/99

amshirif commented 7 months ago

https://github.com/telcoin/telcoin-audit/pull/49

sherlock-admin commented 7 months ago

The protocol team fixed this issue in PR/commit https://github.com/telcoin/telcoin-audit/pull/49.

sherlock-admin commented 6 months ago

The Lead Senior Watson signed-off on the fix.