You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
This repository has been archived by the owner on Oct 1, 2023. It is now read-only.
sherlock-admin opened this issue
Mar 27, 2023
· 0 comments
Labels
DuplicateA valid issue that is a duplicate of an issue with `Has Duplicates` labelMediumA valid Medium severity issueRewardA payout will be made for this issue
mintRollovers() skips a mint when queue[index].assets < relayerFee, while it is entitledShares of a user need to be checked vs relayer's fee as that's the current amount to be relayed to mint.
Vulnerability Detail
queue[index].assets is an initial investment (or a part of it) of the user in a concluded epoch. Assets to be rolled over are represented by previewWithdraw(queue[index].epochId, queue[index].assets) as the point of rolling over is that withdraw wasn't performed yet.
This mean that if Bob the user deposited a minimal amount in the concluded epoch, won big as a result, and wants to invest further, enlisting to rollover. His request will be skipped based on the fact that his initial investment was too small (say it was min allowed and the deposit fee were applied), while that's not relevant as he now has the whole amount won which can pay the relayer fee, i.e. it's a valid queue entry.
Impact
Some mints are skipped, while can be made, which can result in a loss for the corresponding users.
Given that this can happen only on a combination of small initial investment (so current shares are lesser than current relayer fee) and a win in the epoch ended, the severity looks to be medium.
Code Snippet
mintRollovers() treats the initial stake in queue[index].epochId as a maximum possible payment for relayerFee:
function mintRollovers(uint256_epochId, uint256_operations)
externalepochIdExists(_epochId)
epochHasNotStarted(_epochId)
nonReentrant
{
...
while ((index - prevIndex) < (_operations)) {
// only roll over if last epoch is resolvedif (epochResolved[queue[index].epochId]) {
uint256 entitledShares =previewWithdraw(
queue[index].epochId,
queue[index].assets
);
// mint only if user won epoch he is rolling overif (entitledShares > queue[index].assets) {
// skip the rollover for the user if the assets cannot cover the relayer fee instead of revert.
@>if (queue[index].assets < relayerFee) {
index++;
continue;
}
Tool used
Manual Review
Recommendation
Consider using entitledShares to check whether relayer fee can be paid:
if (epochResolved[queue[index].epochId]) {
uint256 entitledShares = previewWithdraw(
queue[index].epochId,
queue[index].assets
);
// mint only if user won epoch he is rolling over
if (entitledShares > queue[index].assets) {
// skip the rollover for the user if the assets cannot cover the relayer fee instead of revert.
- if (queue[index].assets < relayerFee) {+ if (entitledShares < relayerFee) {
index++;
continue;
}
...
}
}
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Labels
DuplicateA valid issue that is a duplicate of an issue with `Has Duplicates` labelMediumA valid Medium severity issueRewardA payout will be made for this issue
iglyx
medium
Valid mints can be skipped in mintRollovers
Summary
mintRollovers() skips a mint when
queue[index].assets < relayerFee
, while it isentitledShares
of a user need to be checked vs relayer's fee as that's the current amount to be relayed to mint.Vulnerability Detail
queue[index].assets
is an initial investment (or a part of it) of the user in a concluded epoch. Assets to be rolled over are represented bypreviewWithdraw(queue[index].epochId, queue[index].assets)
as the point of rolling over is that withdraw wasn't performed yet.This mean that if Bob the user deposited a minimal amount in the concluded epoch, won big as a result, and wants to invest further, enlisting to rollover. His request will be skipped based on the fact that his initial investment was too small (say it was min allowed and the deposit fee were applied), while that's not relevant as he now has the whole amount won which can pay the relayer fee, i.e. it's a valid queue entry.
Impact
Some mints are skipped, while can be made, which can result in a loss for the corresponding users.
Given that this can happen only on a combination of small initial investment (so current shares are lesser than current relayer fee) and a win in the epoch ended, the severity looks to be medium.
Code Snippet
mintRollovers() treats the initial stake in
queue[index].epochId
as a maximum possible payment forrelayerFee
:https://github.com/sherlock-audit/2023-03-Y2K/blob/main/Earthquake/src/v2/Carousel/Carousel.sol#L393-L406
Tool used
Manual Review
Recommendation
Consider using
entitledShares
to check whether relayer fee can be paid:https://github.com/sherlock-audit/2023-03-Y2K/blob/main/Earthquake/src/v2/Carousel/Carousel.sol#L401-L406
Duplicate of #293
The text was updated successfully, but these errors were encountered: