hats-finance / Metrom-0xfdfc6d4ac5807d7460da20a3a1c0c84ef2b9c5a2

Smart contracts for the Metrom project.
GNU General Public License v3.0
0 stars 0 forks source link

Metrom.sol contract misses out on potential yield on Blast #53

Open hats-bug-reporter[bot] opened 5 months ago

hats-bug-reporter[bot] commented 5 months ago

Github username: -- Twitter username: MrPotatoMagic Submission hash (on-chain): 0x46228d890c02499087c99accca32f0cf721b25f754492c0cd4fd29dbd07a121c Severity: medium

Description: Description\ According to the sponsors, the Metrom.sol contract is expected to be deployed on all major EVM chains. One such EVM chain is Blast, which is known for points and campaigns.

Attack Scenario

Blast only has two major ERC20 tokens, WETH and USDB (see explorer). As per the Blast docs, both these tokens always accrue positive yield in accounts holding them, which includes smart contract accounts as well.

From Blast docs:

WETH and USDB accounts have Automatic yield by default for both EOAs and smart contracts.

In our case, if campaign owners use these tokens as their reward tokens, the yield from them is permanetly locked in the Metrom.sol contract and the team is never able to claim it since there is no way to withdraw them.

Severity

The issue is being marked as Medium-severity since although user assets are not at risk, the team loses out on potential yield from their own Metrom.sol smart contract account, thus causing an economic loss. It also falls in line with Hats's Medium-severity description:

Issues that lead to an economic loss but do not lead to direct loss of on-chain assets.

Since WETH and USDB are the only two renowned tokens on Blast, most campaign owners would be using them as reward tokens.

Attachments

We can view the Metrom.sol contract and see that neither of the functions can withdraw this yield.

https://github.com/hats-finance/Metrom-0xfdfc6d4ac5807d7460da20a3a1c0c84ef2b9c5a2/blob/e9d6b1e594d5bb3694bfe68f73399156ebb5d3a4/src/Metrom.sol#L273C1-L311C6

  1. Mitigation

The simplest solution would be to change the Yield mode to be CLAIMABLE instead of the current AUTOMATIC mode. This can be done by calling the configure() function on the ERC20 token contract from an access controlled function in the Metrom.sol contract. Once this is done, call the claim() function on the ERC20 token contract from another access controlled function in the Metrom.sol contract.

mcgrathcoutinho commented 5 months ago

To add on to the impact in this issue, the gas fees spent by campaign owners, eligible LPs and the Metrom team behind calling functions on the Metrom.sol contract is missed out on as potential yield as well (see here). The solution remains the same, change the yield mode to CLAIMABLE and call claimAllGas() as mentioned in the Blast docs linked.

mcgrathcoutinho commented 5 months ago

@luzzif On revisiting this issue, I believe this should be of High-severity as per the Hats docs which classify - long term freezing of unclaimed yield or other assets. as high-severity.

mcgrathcoutinho commented 5 months ago

Hi @luzzif, may I ask why this has been invalidated?

luzzif commented 5 months ago

Absolutely. It's not our intention to deploy to Blast and in general to support rebasing tokens as the introduction of a whitelist on the reward token shows.

mcgrathcoutinho commented 5 months ago

@luzzif Venky told me otherwise that the protocol would also be deployed on Blast, so why is it not being considered in scope now? I think the severity here is high as per the hats documentation since the yield is permanently locked for both tokens as well as the gas.

luzzif commented 5 months ago

Ignoring everything else, this really looks like a duplicate of https://github.com/hats-finance/Metrom-0xfdfc6d4ac5807d7460da20a3a1c0c84ef2b9c5a2/issues/4, with the same root cause.