search

sherlock-audit / 2024-04-titles-judging

1 stars 1 forks source link

0rpse - `collectMintFee` allows users to drain ETH out of `FeeManager.sol` #357

Closed sherlock-admin4 closed 2 months ago

sherlock-admin4 commented 2 months ago

0rpse

medium

collectMintFee allows users to drain ETH out of FeeManager.sol

Summary

Users can utilize collectMintFee function to withdraw funds from FeeManager.sol bypassing withdraw function guarded by admin role checks.

Vulnerability Detail

FeeManager.sol has a withdraw function to remove assets from it by the admin. Users can call collectMintFee to drain ETH as it is open to anyone to call, bypassing withdraw function checks.

Impact

Users can bypass admin checks to withdraw assets from FeeManager.sol.

Code Snippet

https://github.com/sherlock-audit/2024-04-titles/blob/d7f60952df22da00b772db5d3a8272a988546089/wallflower-contract-v2/src/fees/FeeManager.sol#L183-L193

Tool used

Manual Review

Recommendation

Consider adding checks to collectMintFee so that only contracts that should call them have the ability to call it.

Duplicate of #425

0rpse commented 1 month ago

Escalate

The issue of collectMintFee not having proper access control leading to any user being able to route any asset out of FeeManager should be valid

sherlock-admin3 commented 1 month ago

Escalate

The issue of collectMintFee not having proper access control leading to any user being able to route any asset out of FeeManager should be valid

You've created a valid escalation!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

cvetanovv commented 1 month ago

I agree with the escalation, and this issue is a duplicate of #266.

Evert0x commented 1 month ago

Result: High Duplicate of #266

actual severity still depends on 266

sherlock-admin4 commented 1 month ago

Escalations have been resolved successfully!

Escalation status:

0rpse commented 1 month ago

hey @WangSecurity i think you missed this one to duplicate with #425, this was duplicated with #266 this issue mentions ETH so it should be a valid dup of #425

WangSecurity commented 1 month ago

@0rpse thank you for notifying, but #425 will be invalid and this issue has to be invalid as well. Look at #425 for more in-depth explanation as it also applies here.