2024-03-amphor.md

Amphor

Contest Summary

Code under review: 2024-03-amphor (907 nSLOC)

Contest Page: amphor-contest

Findings

[H-1] 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
```diff
_stream.execute(
    _target,
    abi.encodeWithSelector(
        ISablierV2ProxyTarget.withdrawMax.selector,
--      _target,
++      ISablierV2Lockup(_target),
        _id,
        address(this)
    )
);