Skip to content

Malicious user can spam the receive() function in the Minter contract. #64

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
hats-bug-reporter bot opened this issue Sep 10, 2024 · 3 comments
Labels
Invalid - lead auditor invalid This doesn't seem right

Comments

@hats-bug-reporter
Copy link

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

Description:
Description
The receive() function in the Minter contract emits an event for every transfer to the contract.

    // receive() function to handle plain gas token transfers
    receive() external payable {
        emit FundsReceived(msg.sender, msg.value);
    }

It might be used in the UI to handle native transfers to the contract.
However, there is no minimum transfer amount enforced, and the gas fees on OASIS Sapphire are much lower compared to Ethereum. Therefore, a malicious user could spam this event by transferring just 1 wei as many times as possible.

Recommendation
Consider setting a minimum transfer amount for the receive() function.

    receive() external payable {
+      require(msg.value > MIN_TRANSFER);
        emit FundsReceived(msg.sender, msg.value);
    }
@hats-bug-reporter hats-bug-reporter bot added the bug Something isn't working label Sep 10, 2024
@ilzheev
Copy link

ilzheev commented Sep 10, 2024

Events are informational

@ilzheev ilzheev added the invalid This doesn't seem right label Sep 10, 2024
@0xdanial
Copy link

@ilzheev thanks for your comment.

If issue #10 is considered low severity, this should also be considered low as well.

As @0xRizwan mentioned in the comment on issue #10 :

The gas fees on OASIS Sapphire are much lower compared to Ethereum. Malicious users often target low gas fee chains for phishing activities, etc. With such low fees, a user can send multiple transactions. Adopting a minimum deposit check in staking/deposits is a common practice to prevent low depositors, malicious users, or spammers. In my opinion, it's better to implement the validation check to prevent some issues rather than none.

Emitting in the receive() function shows that you want to track received funds, but the report clearly shows that spamming this emit can disrupt the tracking process.

Best regards.

@0xRizwan
Copy link

If user directly transfer native tokens to contract then its user mistake. #10 is different issue. Events are emitted for transparency and nothing to do with spams, etc.

It might be used in the UI to handle native transfers to the contract.

Please check https://testnet.accumulated.finance/stake/rosebeta to understand users interaction with contract specific functions.

@0xRizwan 0xRizwan added Invalid - lead auditor and removed bug Something isn't working labels Sep 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Invalid - lead auditor invalid This doesn't seem right
Projects
None yet
Development

No branches or pull requests

3 participants