hats-finance / SeeR-PM-0x899bc13919880db76edf4ccd72bdfa5dfa666fb7

1 stars 0 forks source link

There is no way to sweep tokens sent by mistake on routers #68

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

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

Github username: -- Twitter username: -- Submission hash (on-chain): 0xaa43d5db43c93c22149eccd5d8d1e98e3e62493be673616076f6a419f384c60a Severity: low

Description: Description

In the GnosisRouter and MainnetRouter contracts, or in the Router.sol they inherit, there is no function to sweep mistakenly sent tokens. This cannot be done for either the accepted tokens like sDAI, xDAI, or any other tokens. Such incidents of mistakenly sent tokens are common in contracts that fall under the "router" category because they handle fund transfer operations.

Proof of Concept (PoC)

Check out any Router contract as example;

GnosisRouter.sol, MainnetRouter.sol and the inherited Router.sol

Impact

Funds sent by mistake will remain stuck forever.

Recommendation

Add a sweep function, this can be either permissioned (only callable by a specific address/addresses) or permissionless depending on the protocol's preference. The function should send the given token to a receiver address for a specified amount or directly the contract balance. Since all transfer operations happen atomically and the router contracts are not designed to hold funds, there should be no issues. However, if desired you can also restrict sweeping for sDAI and xDAI.

greenlucid commented 2 months ago

This is not a vulnerability. It's the user responsibility to not send tokens by mistake to routers, and as long as they use the intended frontends they should be fine. Safety != security

0xpessimist commented 2 months ago

You are absolutely right that this is not a vulnerability, but that was the reason I reported it as Low severity.

It is entirely up to you whether you accept the issue or not, but I still thought it was a good low report in terms of not losing any funds within the system and not losing any potential profits. I tried to base it on Hats Finance's severity assessment, but again no problems if your assessment differs! ❤️

clesaege commented 2 months ago

This is not a vuln but a feature request. I think contracts should be as simple as possible and that adding functions to help people doing mistakes increases their complexity. Moreover, Seer is fully decentralized, there is no owner of those contracts so there isn't any actor who could do that.