Skip to content
This repository has been archived by the owner on Jan 12, 2025. It is now read-only.

Silvermist - Possible stuck of funds in BribeRewarder.sol #256

Closed
sherlock-admin3 opened this issue Jul 15, 2024 · 7 comments
Closed

Silvermist - Possible stuck of funds in BribeRewarder.sol #256

sherlock-admin3 opened this issue Jul 15, 2024 · 7 comments
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Escalation Resolved This issue's escalations have been approved/rejected High A High severity issue. Reward A payout will be made for this issue

Comments

@sherlock-admin3
Copy link
Contributor

sherlock-admin3 commented Jul 15, 2024

Silvermist

High

Possible stuck of funds in BribeRewarder.sol

Summary

If there is a bribe for a voting period but no one votes in it, the reward money will be stuck in the contract since there is no way to withdraw it.

Vulnerability Detail

To incentivize voting for a pool, bribe rewards are created. Rewarder decides for how many voting periods there would be bribes by setting bribe startId and lastId. If he decides he wants to bribe for 5 periods, the funds for rewards are divided into 5 and voters get them after each voting period. This problem is if no one votes for the pool for that bribe period, the rewards for this period will stay in the contract.

For example, if there is only one bribe for period 1 and no one votes in this voting period, that money cannot be used for future bribes and cannot be withdrawn from the rewarder.

Another possible scenario of stuck money is if the rewarder creates bribes for 5 periods and there are only 4 voting periods. The funds for the last period will never be distributed for a user and the rewarder can't withdraw them.

Impact

Stuck of rewards

Code Snippet

https://github.com/sherlock-audit/2024-06-magicsea/blob/42e799446595c542eff9519353d3becc50cdba63/magicsea-staking/src/rewarders/BribeRewarder.sol#L31

Tool used

Manual Review

Recommendation

Implement a withdraw function that allows a trusted user to withdraw the funds if there are no deposits in ended periods.

Duplicate of #172

@github-actions github-actions bot added duplicate Medium A Medium severity issue. labels Jul 21, 2024
@sherlock-admin3 sherlock-admin3 added the Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label label Jul 22, 2024
@sherlock-admin4 sherlock-admin4 changed the title Quick Mahogany Viper - Possible stuck of funds in BribeRewarder.sol Silvermist - Possible stuck of funds in BribeRewarder.sol Jul 29, 2024
@sherlock-admin4 sherlock-admin4 added the Reward A payout will be made for this issue label Jul 29, 2024
@0xSilvermist
Copy link

Escalate

I believe this issue is not a duplicate of #436. Both of the issues describe stuck of rewards, however, the main issue is due to precision loss. This issue has nothing to do with l precision loss and fixing the precision loss will not fix this problem. It should be a dup of #94.

@sherlock-admin3
Copy link
Contributor Author

Escalate

I believe this issue is not a duplicate of #436. Both of the issues describe stuck of rewards, however, the main issue is due to precision loss. This issue has nothing to do with l precision loss and fixing the precision loss will not fix this problem. It should be a dup of #94.

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.

@sherlock-admin4 sherlock-admin4 added the Escalated This issue contains a pending escalation label Aug 1, 2024
@0xSmartContract
Copy link
Collaborator

Code snippet does not show specific code, it shows the contract, like a general hunch, the problem is not fully explained, there is no code, this seems to have been effective in the judge's wrong decision

I am not sure if the issue meets the minimum criteria to not be Invalid.

IX. Duplication rules:
The duplication rules assume we have a "target issue", and the "potential duplicate" of that issue needs to meet the >following requirements to be considered a duplicate.

Identify the root cause
Identify at least a Medium impact
Identify a valid attack path or vulnerability path
Fulfills other submission quality requirements

@WangSecurity
Copy link

I believe this report has all the requirements, it has identified the root cause, has high impact and a vulnerability path. Hence, I believe it's a sufficient duplicate of #94.
Planning to accept the escalation and duplicate with #94.

But #94 is currently a duplicate of the escalated #164. If #94 remains a duplicate of #164, then this escalation will still be accepted, but the report will be duplicated with #164.

@WangSecurity
Copy link

WangSecurity commented Aug 11, 2024

UPD: #94 is going to be duplicated with #172, this issue will be duplicated with #172 as well, after #164 escalation is resolved

@WangSecurity WangSecurity added High A High severity issue. and removed Medium A Medium severity issue. labels Aug 19, 2024
@WangSecurity
Copy link

WangSecurity commented Aug 19, 2024

Result:
High
Duplicate of #172

@sherlock-admin2
Copy link

sherlock-admin2 commented Aug 19, 2024

Escalations have been resolved successfully!

Escalation status:

@sherlock-admin3 sherlock-admin3 removed the Escalated This issue contains a pending escalation label Aug 19, 2024
@sherlock-admin4 sherlock-admin4 added the Escalation Resolved This issue's escalations have been approved/rejected label Aug 19, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Escalation Resolved This issue's escalations have been approved/rejected High A High severity issue. Reward A payout will be made for this issue
Projects
None yet
Development

No branches or pull requests

6 participants