Skip to content
This repository has been archived by the owner on Oct 1, 2023. It is now read-only.

datapunk - Owner might call createEpoch directly by mistake instead of createEpochWithEmission, and there are no way to set emissions properly #505

Closed
sherlock-admin opened this issue Mar 28, 2023 · 3 comments
Labels
Low/Info A valid Low/Informational severity issue Non-Reward This issue will not receive a payout Sponsor Confirmed The sponsor acknowledged this issue is valid

Comments

@sherlock-admin
Copy link
Contributor

datapunk

medium

Owner might call createEpoch directly by mistake instead of createEpochWithEmission, and there are no way to set emissions properly

Summary

for carousal, is there a chance the owner might call createEpoch directly by mistake instead of createEpochWithEmissions. Once the epoch is created, it does not look like there is a way to set the proper Emissions anymore

Vulnerability Detail

carousal inherits from vault, but does not override some functions explicitly, which gives room for the owner to create new epochs through createEpoch instead of createEpochWithEmissions. Since OnlyFactory can call setEmissions and the factory contract only makes such call in createEpochWithEmissions, there is no way to revise the emission parameters for the the epoch.

Impact

Lost emission revenue for treasury

Code Snippet

https://github.com/sherlock-audit/2023-03-Y2K/blob/main/Earthquake/src/v2/Carousel/CarouselFactory.sol#L132
https://github.com/sherlock-audit/2023-03-Y2K/blob/main/Earthquake/src/v2/Carousel/Carousel.sol#L591

Tool used

Manual Review

Recommendation

Explicitly disable createEpoch function in carousal contract.

@github-actions github-actions bot added Medium A valid Medium severity issue Has Duplicates A valid issue with 1+ other issues describing the same vulnerability labels Apr 3, 2023
@3xHarry 3xHarry added the Sponsor Confirmed The sponsor acknowledged this issue is valid label Apr 4, 2023
@3xHarry
Copy link

3xHarry commented Apr 4, 2023

Agree with this issue, easiest would be to just revert to createEpoch/createNewMarket and separate the creation of markets based on deployed factory.
More gas efficient would be to not inherit VaultFactoryV2 and modify the createEpoch function interface to be able to set Emissions.

3xHarry added a commit to Y2K-Finance/Earthquake that referenced this issue Apr 7, 2023
@dmitriia dmitriia added Low/Info A valid Low/Informational severity issue and removed Has Duplicates A valid issue with 1+ other issues describing the same vulnerability Medium A valid Medium severity issue labels Apr 9, 2023
@dmitriia
Copy link
Collaborator

dmitriia commented Apr 9, 2023

Low as no loss of funds and requires operator mistake

@dmitriia dmitriia closed this as completed Apr 9, 2023
@sherlock-admin sherlock-admin added Has Duplicates A valid issue with 1+ other issues describing the same vulnerability Non-Reward This issue will not receive a payout labels Apr 11, 2023
@3xHarry
Copy link

3xHarry commented Apr 11, 2023

fix PR: Y2K-Finance/Earthquake#129

@sherlock-admin sherlock-admin removed the Has Duplicates A valid issue with 1+ other issues describing the same vulnerability label Apr 28, 2023
3xHarry added a commit to Y2K-Finance/Earthquake that referenced this issue May 10, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Low/Info A valid Low/Informational severity issue Non-Reward This issue will not receive a payout Sponsor Confirmed The sponsor acknowledged this issue is valid
Projects
None yet
Development

No branches or pull requests

3 participants