Tricko - Holder of `CouncilMember` NFT can DoS the `CouncilMember` contract.

[sherlock-admin]

Tricko

high

Holder of CouncilMember NFT can DoS the CouncilMember contract.

Summary

Because SablierV2ProxyTarget.withdrawMax() reverts when withdrawable tokens from the corresponding stream is zero, a malicious council member can exploit this vulnerability using CouncilMember.retrieve() to DoS the majority of CouncilMember functionalities such as CouncilMember.claim(), CouncilMember.mint(), CouncilMember.burn(), and CouncilMember.removeFromOffice().

Vulnerability Detail

Before any significant state changes in the CouncilMember contract, the internal method _retrieve() is called. The purpose of _retrieve() is to retrieve funds from the Sablier stream and distribute them among council members (holders of CouncilMember NFTs). To obtain the accrued funds from the stream, it utilizes the SablierV2ProxyTarget.withdrawMax() function, internally calling SablierV2Lockup.withdrawMax() and SablierV2Lockup.withdraw(). As the name suggests, this function withdraws all available funds from the specified stream.

If SablierV2ProxyTarget.withdrawMax() is called again in the same block, the amount of withdrawable tokens will be zero because of the previous call to withdrawMax(). However, examining the Sablier codebase reveals that if SablierV2Lockup.withdraw() is called with amount = 0, the call reverts with the error Errors.SablierV2Lockup_WithdrawAmountZero(streamId).

Therefore, if CouncilMember._retrieve() is called once in a block, withdrawing all available funds from the stream, subsequent calls to CouncilMember._retrieve() within the same block will result in a revert. A malicious council member can exploit this behavior to indefinitely execute a DoS attack on the majority of CouncilMember functionality, affecting methods such as CouncilMember.claim(), CouncilMember.mint(), CouncilMember.burn(), and CouncilMember.removeFromOffice().

The DoS attack would proceed as follows:

1. The address with GOVERNANCE_COUNCIL_ROLE calls CouncilMember.removeFromOffice(...).
2. The malicious council member observes the transaction by GOVERNANCE_COUNCIL_ROLE in the mempool and attempts to front-run it by calling CouncilMember.retrieve().

If the front-run is successful and retrieve() call is executed first, the remaining funds on the stream will be zero during that block. Consequently, the transaction by GOVERNANCE_COUNCIL_ROLE reverts due to the issues described above.

It's essential to note that the attack's cost is minimal, requiring only the gas to call the CouncilMember.retrieve() function. The malicious council member can repetitively execute this attack every block, continuously DoSing the CouncilMember contract. Even attempts by GOVERNANCE_COUNCIL_ROLE to remove the malicious council member can be thwarted, as the attacker can also DoS their own removal.

Impact

A malicious council member can indefinitely DoS the majority of CouncilMember functionality.

Code Snippet

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

Tool used

Manual Review

Recommendation

Consider using the withdrawableAmountOf() function exposed by SablierV2Lockup to verify if withdrawable amount is non-zero during the execution of CouncilMember._retrieve(). If the withdrawable amount is zero, consider bypassing the fund withdrawal process.

Duplicate of #47

[sherlock-admin2]

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

takarez commented:

valid because { After carefull review; this turns out to be a valid issue as the fron-tun is possible despite the onlyAuthorized modifier in the retrieve function; the issue 092 explained how a malicious user with NFT can bypassed the onlyAuthorized modifier and call the retrieve and im also making it the best report among the others even though its a bit different than the others; but the underlying cause of zero balance is the same}