sherlock-audit / 2024-01-telcoin-judging

6 stars 5 forks source link

eeshenggoh - Overinflated rewards updated due to flaw in calling SablierV2ProxyTarget #111

Closed sherlock-admin2 closed 7 months ago

sherlock-admin2 commented 7 months ago

eeshenggoh

high

Overinflated rewards updated due to flaw in calling SablierV2ProxyTarget

Summary

CouncilMember::_retrieve() is used to retrieve and distribute TELCOIN to council members based on the stream from _target. It uses a Sabiler PRBproxy to withdraw tokens to the CouncilMembers contract. The problem lies in not the wrong input parameter.

Vulnerability Detail

The input parameter to call ISablierV2ProxyTarget::withdrawMax() implemented in CouncilMember::_retrieve() uses a proxy with an encoded selector. The call to the withdrawal will always not be executed because of the input variable using the wrong address.

The ISablierV2ProxyTarget Line 74 etherscan Implementation LoC

    function withdrawMax(
        ISablierV2Lockup lockup, // @audit require ISablierV2Lockup instance
        uint256 streamId,
        address to
    ) external;

Impact

The call to the Sablier's Target will never be executed, hence overinflated rewards are updated to the users.

Code Snippet

https://github.com/sherlock-audit/2024-01-telcoin/blob/main/telcoin-audit/contracts/sablier/core/CouncilMember.sol#L270-L279C1

    _stream.execute(
        _target,
        abi.encodeWithSelector(
            ISablierV2ProxyTarget.withdrawMax.selector,
            _target, //@audit wrong type
            _id,
            address(this)
        )
    );

Tool used

Manual Review

Recommendation

_stream.execute(
    _target,
    abi.encodeWithSelector(
        ISablierV2ProxyTarget.withdrawMax.selector,
--      _target,
++      ISablierV2Lockup(_target),
        _id,
        address(this)
    )
);

Duplicate of #139

sherlock-admin2 commented 7 months ago

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

takarez commented:

valid because { This is valid and a dupp of 086; it should have been a high severity; but due the impact mentioned in issue 086 im making it meduim because it will likely be discoverd before even talking about the funds being in the contract i believe}