zzykxx - A round can be canceled after a random number has been drawn in a specific edge case

[sherlock-admin]

zzykxx

medium

A round can be canceled after a random number has been drawn in a specific edge case

Summary

There's an edge case in which the protocol fairness can be undermined by canceling a round after a random number has been drawn.

Vulnerability Detail

The cancelAfterRandomnessRequest() function allows anybody to cancel the current round if the round status is Drawn and at least 1 day (24 hours) passed since the randomness request.

To understand the root cause of this issue there's two things to keep in mind:

1. A round can be cancelled if least 1 day (24 hours) passed since the randomness request
2. If the LINK balance of the chainlink VRF subscription is below the minimum required a request for randomness will remain pending for up to 24 hours

This might look fine in theory but in practice there's a small time window in which an user can cancel a round if the random number sent by chainlink is not in their favor.

POC

Here's how it works:

1. A request for randomness is sent to chainlink VRF via _drawWinner()
2. The LINK balance of the subscription is below the minimum required to fulfill the request
3. Alice who is participating in the round notices this and funds the chainlink subscription with LINKs right before the 24 hours mark
4. The chainlink node sends the randomness response to YOLOV2 but the transaction stays pending in the mempool for some minutes
5. Alice monitors the mempool and checks if the random number sent by chainlink is in her favor (ex. she's the winner) or not
6. If she's not the winner she can fronturn the randomness response and cancel the round, she can do this because the 24 hours mark is passed from the point of view of cancelAfterRandomnessRequest()

Important to note is that the delay between 4. and .6 can happen because:

• The attackers times everything perfectly
• The network is congested
• The attacker fills blocks on purpose to delay the chainlink response

Impact

Although this requires some preconditions and is unlikely to happen, there's a possibility of undermining the fairness of the protocol which is one of his main selling points:

The YOLO system is designed to operate a transparent, fair, and decentralized game directly on the Ethereum blockchain.

Code Snippet

• cancelAfterRandomnessRequest()

Tool used

Manual Review

Recommendation

Increase the time window after which cancelAfterRandomnessRequest() can be called by at least 10 minutes on mainnet and potentially more on cheaper chains:

if (block.timestamp < round.drawnAt + 1 days + 10*60) {
    revert DrawExpirationTimeNotReached();
}

[0xhiroshi]

1. This is too circumstantial
2. How does adding 10 minutes make any difference?

[zzykxx]

Escalate

I agree that this is circumstantial but I also think the impact is critical for the protocol given that the whole system is built on the principle of being a fair lottery. Because of low likelihood but high impact I believe this should be considered as a valid.

Adding 10 minutes of delay on mainnet means an attacker that funded the chainlink subscription contract has to hope the transaction stays pending for more than 10 minutes to be able to execute the attack. The fix is straightforward and just requires increasing the time after which cancelAfterRandomnessRequest() can be called. I don't see why the protocol should take the risk of having this "exploit" occur when it can be prevented easily.

[sherlock-admin2]

Escalate

I agree that this is circumstantial but I also think the impact is critical for the protocol given that the whole system is built on the principle of being a fair lottery. Because of low likelihood but high impact I believe this should be considered as a valid.

Adding 10 minutes of delay on mainnet means an attacker that funded the chainlink subscription contract has to hope the transaction stays pending for more than 10 minutes to be able to execute the attack. The fix is straightforward and just requires increasing the time after which cancelAfterRandomnessRequest() can be called. I don't see why the protocol should take the risk of having this "exploit" occur when it can be prevented easily.

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.

[0xhiroshi]

@zzykxx Anyone can top up the Chainlink subscription within the 24 hours, not just Alice.

[nevillehuang]

Invalid, anybody can top up LINK (even admin) at anytime to prevent this from happening, so this is a non-issue. Adding a buffer also do not mitigate the issue.

[ArnieSec]

Escalate

I would kindly like to escalate

As a default we cannot rely on users of the protocol to top up LINK in order to keep protocol functioning. Additionally users seek profit, the chance that a user will willingly buy LINK then send that LINK for top up is extremely unlikely. This is akin to a lottery buyer sending money to the organizer, it just isn't feasible or even likely.

Furthermore, the readme state that YOLO will be deployed on

Mainnet
Arbitrum
Base
Potentially any EVM compatible L2 (Including ZK L2s)

If we include EVM compatible L2 the amount of yolo instances will be over 8. The likeliness that a Subscription stays under funded for 24 hours is likely now.

Even more we are assuming that the admin/owner is responsible for LINK top up, this was never stated in the readme, here are all the listed action of the admin

DEFAULT_ADMIN_ROLE:
cancel(uint256 numberOfRounds) - cancel 1 or more rounds, starting from the current round, regardless of round state. No validations are performed.
togglePaused() - Pause or unpause contract
toggleOutflowAllowed() - Allow or disallow token outflow
updateRoundDuration(uint40 _roundDuration) - Update round duration. Capped at 1 hour
updateSignatureValidityPeriod(uint40 _signatureValidityPeriod) - Update Reservoir oracle signature validity period
updateValuePerEntry(uint96 _valuePerEntry) - Update value (in ETH wei) per entry
updateProtocolFeeRecipient(address _protocolFeeRecipient) - Update address that receives protocol fee
updateProtocolFeeBp(uint16 _protocolFeeBp) - Update fee percentage. Capped at 25%
updateProtocolFeeDiscountBp(uint16 _protocolFeeDiscountBp) - Update discount percentage if fee if paid in LOOKS. Capped at 100%
updateMaximumNumberOfParticipantsPerRound(uint40 _maximumNumberOfParticipantsPerRound) - Update maximum number of participants per round. Minimum value of 2.
updateReservoirOracle(address _reservoirOracle) - Update reservoir oracle address.
updateERC20Oracle(address _erc20Oracle) - Update ERC20 TWAP oracle address

OPERATOR:
updateCurrenciesStatus(address[] calldata currencies, bool isAllowed) - add or remove token/collection addresses from the whitelist

Therefor we cannot assume that the person who is responsible for top up is trusted. As a default if an actor is not named, we assume him to not be trusted. This is the case here.

@nevillehuang Adding a 10 min buffer does help to mitigate the issue. I kindly ask for you to read the Chainlink documentation. Pending chainlink orders expire after 24 hours, if we add 10 minutes to the 24 hour delay, we can now be sure the pending request would have expired by that time.

Each subscription must maintain a minimum balance to fund requests from consuming contracts. This minimum balance requirement serves as a buffer against gas volatility by ensuring that all your requests have more than enough funding to go through. If your balance is below the minimum, your requests remain pending for up to 24 hours before they expire. After you add sufficient LINK to a subscription, pending requests automatically process as long as they have not expired.

https://docs.chain.link/vrf/v2/subscription

The argument against this issue is that it can be prevented. This is not enough reason to disqualify and issue especially since there is a scenario where a loss of funds is possible and this issue allows gaming a user odds, which is complete breaking of the contracts core functionality.

We are also disqualifying this issue on the basis that the user will do the right thing and top up the LINK, using this same logic again, we must assume that issue 18 must also be invalid if we assume that the user will always do the right thing and not deposit with Value 0.

Now according to sherlock docs, what makes a valid high issue?

Definite loss of funds without (extensive) limitations of external conditions.
Inflicts serious non-material losses (doesn't include contract simply not working).

here we do have a definite loss of funds without extensive limitations of external conditions. There is only 1 external condition that needs to be met, and that is that LINK is not topped up. This is NOT extensive so therefore this issue fits the high severity criteria without a doubt.

I have proven why this issue is a valid high according to sherlocks own documentation. I ask kindly that we can judge this issue correctly, thanks.

[sherlock-admin2]

Escalate

I would kindly like to escalate

As a default we cannot rely on users of the protocol to top up LINK in order to keep protocol functioning. Additionally users seek profit, the chance that a user will willingly buy LINK then send that LINK for top up is extremely unlikely. This is akin to a lottery buyer sending money to the organizer, it just isn't feasible or even likely.

Furthermore, the readme state that YOLO will be deployed on

Mainnet
Arbitrum
Base
Potentially any EVM compatible L2 (Including ZK L2s)

If we include EVM compatible L2 the amount of yolo instances will be over 8. The likeliness that a Subscription stays under funded for 24 hours is likely now.

Even more we are assuming that the admin/owner is responsible for LINK top up, this was never stated in the readme, here are all the listed action of the admin

DEFAULT_ADMIN_ROLE:
cancel(uint256 numberOfRounds) - cancel 1 or more rounds, starting from the current round, regardless of round state. No validations are performed.
togglePaused() - Pause or unpause contract
toggleOutflowAllowed() - Allow or disallow token outflow
updateRoundDuration(uint40 _roundDuration) - Update round duration. Capped at 1 hour
updateSignatureValidityPeriod(uint40 _signatureValidityPeriod) - Update Reservoir oracle signature validity period
updateValuePerEntry(uint96 _valuePerEntry) - Update value (in ETH wei) per entry
updateProtocolFeeRecipient(address _protocolFeeRecipient) - Update address that receives protocol fee
updateProtocolFeeBp(uint16 _protocolFeeBp) - Update fee percentage. Capped at 25%
updateProtocolFeeDiscountBp(uint16 _protocolFeeDiscountBp) - Update discount percentage if fee if paid in LOOKS. Capped at 100%
updateMaximumNumberOfParticipantsPerRound(uint40 _maximumNumberOfParticipantsPerRound) - Update maximum number of participants per round. Minimum value of 2.
updateReservoirOracle(address _reservoirOracle) - Update reservoir oracle address.
updateERC20Oracle(address _erc20Oracle) - Update ERC20 TWAP oracle address

OPERATOR:
updateCurrenciesStatus(address[] calldata currencies, bool isAllowed) - add or remove token/collection addresses from the whitelist

Therefor we cannot assume that the person who is responsible for top up is trusted. As a default if an actor is not named, we assume him to not be trusted. This is the case here.

@nevillehuang Adding a 10 min buffer does help to mitigate the issue. I kindly ask for you to read the Chainlink documentation. Pending chainlink orders expire after 24 hours, if we add 10 minutes to the 24 hour delay, we can now be sure the pending request would have expired by that time.

Each subscription must maintain a minimum balance to fund requests from consuming contracts. This minimum balance requirement serves as a buffer against gas volatility by ensuring that all your requests have more than enough funding to go through. If your balance is below the minimum, your requests remain pending for up to 24 hours before they expire. After you add sufficient LINK to a subscription, pending requests automatically process as long as they have not expired.

https://docs.chain.link/vrf/v2/subscription

The argument against this issue is that it can be prevented. This is not enough reason to disqualify and issue especially since there is a scenario where a loss of funds is possible and this issue allows gaming a user odds, which is complete breaking of the contracts core functionality.

We are also disqualifying this issue on the basis that the user will do the right thing and top up the LINK, using this same logic again, we must assume that issue 18 must also be invalid if we assume that the user will always do the right thing and not deposit with Value 0.

Now according to sherlock docs, what makes a valid high issue?

Definite loss of funds without (extensive) limitations of external conditions.
Inflicts serious non-material losses (doesn't include contract simply not working).

here we do have a definite loss of funds without extensive limitations of external conditions. There is only 1 external condition that needs to be met, and that is that LINK is not topped up. This is NOT extensive so therefore this issue fits the high severity criteria without a doubt.

I have proven why this issue is a valid high according to sherlocks own documentation. I ask kindly that we can judge this issue correctly, thanks.

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.

[0xhiroshi]

Escalate

I would kindly like to escalate

As a default we cannot rely on users of the protocol to top up LINK in order to keep protocol functioning. Additionally users seek profit, the chance that a user will willingly buy LINK then send that LINK for top up is extremely unlikely. This is akin to a lottery buyer sending money to the organizer, it just isn't feasible or even likely.

Furthermore, the readme state that YOLO will be deployed on

Mainnet
Arbitrum
Base
Potentially any EVM compatible L2 (Including ZK L2s)

If we include EVM compatible L2 the amount of yolo instances will be over 8. The likeliness that a Subscription stays under funded for 24 hours is likely now.

Even more we are assuming that the admin/owner is responsible for LINK top up, this was never stated in the readme, here are all the listed action of the admin

DEFAULT_ADMIN_ROLE:
cancel(uint256 numberOfRounds) - cancel 1 or more rounds, starting from the current round, regardless of round state. No validations are performed.
togglePaused() - Pause or unpause contract
toggleOutflowAllowed() - Allow or disallow token outflow
updateRoundDuration(uint40 _roundDuration) - Update round duration. Capped at 1 hour
updateSignatureValidityPeriod(uint40 _signatureValidityPeriod) - Update Reservoir oracle signature validity period
updateValuePerEntry(uint96 _valuePerEntry) - Update value (in ETH wei) per entry
updateProtocolFeeRecipient(address _protocolFeeRecipient) - Update address that receives protocol fee
updateProtocolFeeBp(uint16 _protocolFeeBp) - Update fee percentage. Capped at 25%
updateProtocolFeeDiscountBp(uint16 _protocolFeeDiscountBp) - Update discount percentage if fee if paid in LOOKS. Capped at 100%
updateMaximumNumberOfParticipantsPerRound(uint40 _maximumNumberOfParticipantsPerRound) - Update maximum number of participants per round. Minimum value of 2.
updateReservoirOracle(address _reservoirOracle) - Update reservoir oracle address.
updateERC20Oracle(address _erc20Oracle) - Update ERC20 TWAP oracle address

OPERATOR:
updateCurrenciesStatus(address[] calldata currencies, bool isAllowed) - add or remove token/collection addresses from the whitelist

Therefor we cannot assume that the person who is responsible for top up is trusted. As a default if an actor is not named, we assume him to not be trusted. This is the case here.

@nevillehuang Adding a 10 min buffer does help to mitigate the issue. I kindly ask for you to read the Chainlink documentation. Pending chainlink orders expire after 24 hours, if we add 10 minutes to the 24 hour delay, we can now be sure the pending request would have expired by that time.

Each subscription must maintain a minimum balance to fund requests from consuming contracts. This minimum balance requirement serves as a buffer against gas volatility by ensuring that all your requests have more than enough funding to go through. If your balance is below the minimum, your requests remain pending for up to 24 hours before they expire. After you add sufficient LINK to a subscription, pending requests automatically process as long as they have not expired.

https://docs.chain.link/vrf/v2/subscription

The argument against this issue is that it can be prevented. This is not enough reason to disqualify and issue especially since there is a scenario where a loss of funds is possible and this issue allows gaming a user odds, which is complete breaking of the contracts core functionality.

We are also disqualifying this issue on the basis that the user will do the right thing and top up the LINK, using this same logic again, we must assume that issue 18 must also be invalid if we assume that the user will always do the right thing and not deposit with Value 0.

Now according to sherlock docs, what makes a valid high issue?

Definite loss of funds without (extensive) limitations of external conditions.
Inflicts serious non-material losses (doesn't include contract simply not working).

here we do have a definite loss of funds without extensive limitations of external conditions. There is only 1 external condition that needs to be met, and that is that LINK is not topped up. This is NOT extensive so therefore this issue fits the high severity criteria without a doubt.

I have proven why this issue is a valid high according to sherlocks own documentation. I ask kindly that we can judge this issue correctly, thanks.

1. We top up our subscriptions regularly, with bots monitoring our subscriptions' balances. Saying we can't be trusted to do it because it's not stated in the README is quite a statement. Our product would have failed within days if we don't actively top up our subscriptions. Our V1 product has been running for months.

2. After 24 hours, Chainlink actually can still manually fulfill pending randomness requests so technically adding 10 minutes on top of the 24 hours doesn't stop it from happening.

[ArnieSec]

1. So a bot is responsible for top up, given that the bots code is not in scope, we cannot refute a finding with info that was not present during the length of this audit, this is a sherlock standard. Additionally unless we assume infinite funds, we cannot assure that the bot will always top up LINK. That bot at some point will also run out of funds, and who is in charge of topping up that bots balance?

Just like historical decisions cannot influence a current finding, it is also true that YOLO v1 past historical performance cannot be used to refute this issue. This is stated in the sherlock docs, and just because this exploit has not yet happened does not mean it will never happen

Historical decisions are not considered sources of truth

2. Manually fulfilling orders can only be done by the admin/owner of the subscription. Therefore the 10 minute buffer does mitigate the issue since the attacker can no longer manipulate the pending or not pending state at will.

@0xhiroshi I am not trying to be arrogant or come off in the wrong way. We must follow procedures and rules. I have again through sherlocks own docs described why this issue is valid.

I ask the judge does not judge this issue based on bias but rather logic and what is considered a valid issue according to the sherlock documentation. Strict adherence to rules is what is fair for everyone involved.

[0xhiroshi]

We use the bot to monitor our balances but we top up manually, but even if do have a bot that automatically tops up our subscriptions, your argument that it can run out of balance is a stretch. I can then say we will always have someone to top up the bot, and then you can say something like "what if the person responsible for topping up the bot is sick, or what if the computer with LINK has no battery"? This can go on and on and deviates from the protocol itself. And since it's no longer about the smart contract but whether someone can top it up in 24 hours, is this really in scope? Also, 24 hours is a very long time. We are not talking about the round can be manipulated if we don't top up the subscription within a block, but a full day.

[ArnieSec]

The fact is that in scope contracts have an edge case which can cause loss of funds for users and breaks key functionality given a certain condition. This is to say, given a certain condition, the issue is valid.

From sherlock docs we can observe that a issue is a valid high if it causes loss of funds in certain condition, these conditions cannot be extensive.

Definite loss of funds without (extensive) limitations of external conditions

The external condition is not extensive, the external condition of this issue is that LINK is not topped up for 24 hours. Ability to avoid an external condition is not sufficient to refute the issue.

Because sherlock allows highs to be submitted even if it includes an external condition(as long as this external condition is not extensive) which this issue does, the issue is valid given that the external condition is met and is not extensive which is the case here.

This is my final comment :)

[zzykxx]

I also add my final observations about the discussion:

1. The fact that this issue can be prevented can't be used as an argument to invalidate it. The issue has to be known in the first place to be prevented. This is like saying inflation attacks on vaults are invalid because anybody can send funds directly to the vault to prevent the attack.

2. The only pre-condition here is that the subscription chainlink contract doesn't have enough funds and does not get topped up for 24 hours. Unlikely but possible.

3. It should be considered a valid medium but not a high because there are pre-conditions but the impact is critical if executed.

[0xhiroshi]

@zzykxx

To make this even more circumstantial, let's say we follow your suggestion and add 10 minutes to the 24 hours deadline before the round can be cancelled, and because the network is so congested, the Chainlink randomness fulfillment that was sent by the 24 hours mark (attacker topped up the subscription around that time and the transaction was confirmed) stays in the mempool for 10+ minutes, and the current block timestamp is passed the round's drawn at + 24 hours + 10 minutes. So now the round can be cancelled with a Chainlink VRF response in the mempool and we are in the exact same frontrun situation. What do you suggest?

[ArnieSec]

@0xhiroshi chainlink nodes bump the gas on tx in the mempool after a predefined number of blocks, the example used in documentation was 3 blocks or around 36 seconds. The gas bump is used to ensure that a chain link tx does not stay in the mempool for long. So the buffer should be sufficient in most cases to mitigate the risk. Unless we expect gas prices to rise every block for 30 minutes straight. Which is 100x more unlikely to happen than someone forgetting to top up LINK. Once again when doing security we plan for the worst case scenario, because we are human and are therefore not perfect. We can be expected to slip up from time to time. Forgetting to top up LINK is a likely scenario. We cannot always assume that everything goes as planned everyday all day for the rest of YOLOV2 life cycle. It is best for us to take in worst case scenarios and implement fixes to mitigate these risks if they ever were to happen.

[0xhiroshi]

If we have to be purely hypothetical, there is still no guarantee for the transaction to be confirmed on time even if Chainlink bumps the gas price. The gas price can spike to a level higher than our gas lane after the fulfilment is submitted to the network, correct me if I am wrong but I don't think Chainlink can bump the gas price even further as it has already reached the upper limit of the gas lane?

imo having the gas price to rise above 200 gwei on mainnet (we are not going for more than 200 gwei because we will lose money if we go for 500 or 1000 gwei) for more than 10 minutes can happen easily if let's say there is a price crash and a bunch of people are getting liquidated on-chain. I can say it's even more likely than someone forgetting to top up our Chainlink subscription. It's all subjective here and we can't really say what's "100x" more likely or not.

[ArnieSec]

Yeah that's true. But at the end of the day I have described why according to Sherlock documentation that this is a valid issue. You have tried to refute finding with what ifs and no clear reference to documentation to support your dispute. If you really think that everything will be perfect and the team will never forget to top up link.

Then what is the point of having the
cancelAfterRandomnessRequest() function. Just remove it if you are so sure everything will go according to plan. It's obvious someone who coded the protocol added this function in case things do go wrong, therefore the own protocol thinks the risk is high enough for them to code up this function and include it in the protocol.

[ArnieSec]

I would accept this issue as invalid if it is disputed using/referencing either the readme or Sherlock documentation. It is expected of watsons to use logic/ documentation to support our finding. It should therefore be the same way for the protocol team. Please use logic and the documentation/rules of Sherlock to dispute the finding. Not personal beliefs and what ifs.

[0xhiroshi]

Yeah that's true. But at the end of the day I have described why according to Sherlock documentation that this is a valid issue. You have tried to refute finding with what ifs and no clear reference to documentation to support your dispute. If you really think that everything will be perfect and the team will never forget to top up link.

Then what is the point of having the cancelAfterRandomnessRequest() function. Just remove it if you are so sure everything will go according to plan. It's obvious someone who coded the protocol added this function in case things do go wrong, therefore the own protocol thinks the risk is high enough for them to code up this function and include it in the protocol.

I don't see how this issue itself isn't also based on a lot of "what ifs" lining up together and even with the proposed fix it can still break in theory. You are getting personal here, saying it's based on my personal belief.

[ArnieSec]

It's not my intent to be personal, but if Sherlock allows a valid issue if they contain an external condition, I don't see why you think the issue is invalid.

Here is a snippet from Sherlock documentation

Causes a loss of funds but requires certain external conditions or specific states, or a loss is highly constrained. The losses must exceed small, finite amount of funds, and any amount relevant based on the precision or significance of the loss.

This is criteria for medium, which this issue fits perfectly. I now see your argument is correct to disqualify this issue as high. But it is definitely a medium. Sorry if I came off the wrong way, I am just very passionate about this space.

[0xhiroshi]

It's not my intent to be personal, but if Sherlock allows a valid issue if they contain an external condition, I don't see why you think the issue is invalid.

Here is a snippet from Sherlock documentation

Causes a loss of funds but requires certain external conditions or specific states, or a loss is highly constrained. The losses must exceed small, finite amount of funds, and any amount relevant based on the precision or significance of the loss.

This is criteria for medium, which this issue fits perfectly. I now see your argument is correct to disqualify this issue as high. But it is definitely a medium. Sorry if I came off the wrong way, I am just very passionate about this space.

Let's have a constructive conversation starting from here.

If we really want to make sure it's absolute airtight, just adding 10 minutes isn't going to fix the problem, just a smaller chance of having it. Is there a better solution than adding more and more buffer? We can't lock user funds for too long.

[ArnieSec]

@0xhiroshi yes I think making the function cancelAfterRandomnessRequest a privileged function will solve this issue. The admin can check the chainlink dashboard to ensure there is no pending/ sent tx in mempool and then he can then call cancelAfterRandomnessRequest himself. This should mitigate this issue 100%.

[0xhiroshi]

@0xhiroshi yes I think making the function cancelAfterRandomnessRequest a privileged function will solve this issue. The admin can check the chainlink dashboard to ensure there is no pending/ sent tx in mempool and then he can then call cancelAfterRandomnessRequest himself. This should mitigate this issue 100%.

On the other hand I am worried it can be said that there is a possibility of us holding up users' fund for a round that should be cancelled for not executing cancelAfterRandomnessRequest.

[ArnieSec]

Maybe a 1 hour buffer added to cancelAfterRandomnessRequest is sufficient. This will only hold up funds for an extra hour. At this point the chance that a chainlink tx has been in mempool is extremely low. Also adding comments to the function explaining that after 24 hours, chainlink values should not be considered.

[ArnieSec]

In this way if we cancel a round after 25 hours and there is a randomness request in mempool, we can assure the users that this was expected behavior.

[0xhiroshi]

https://github.com/LooksRare/contracts-yolo/pull/208

[nevillehuang]

@ArnieGod @zzykxx In my personal opinion, since admins are trusted, if they want the game to stop by not fuifilling the following precondition, it is a valid centralization risk consideration. This is not even considering any users can just continue the game anyways.

2. The LINK balance of the subscription is below the minimum required to fulfill the request

More importantly, even if you increase it to 25 hours, there is always the argument for the same scenario that can happen if chainlink response doesn't return by 25 hours, than the front-running issue can happen again and so on and so forth, where the possibility is endless. Additionally, for cancelled rounds, users can simply withdraw their deposits for cancelled deposits via withdrawDeposit()

Hence, I believe this issue should be low severity. Also the difference between this and #43 is the likelihood and edge case scenarios for this to occur.

[ArnieSec]

This is just not true, respectfully your comment shows clearly you don't have full context or understanding of the issue at hand, because what your stating is not even true, front running will be impossible if the chainlink vrf didn't return a value by 25 hours because there will be no tx to frontrun. I kindly ask the head judge to read the duplicate of this report for better understanding #102 that duplicate highlights impact and issue more thoroughly. The impact is breaking core functionality and also a loss of funds. And I know historical decisions do not matter but the exact same report was deemed high severity when reported by trust in the forgeries contest on c4. I can further explain the issue in detail if necessary, if I were asked to be 100% unbiased I would without a doubt count this issue a valid medium at the very least, if not there is also some argument for a high.

[nevillehuang]

@ArnieGod Can you point me to the c4 issue? I will have a look and rereview my comments.

Could you correct me about why there will be no tx to front-run if you increase expiry to 25 hours before allowing cancellation?

[ArnieSec]

@nevillehuang code-423n4/2022-12-forgeries-findings#272

[nevillehuang]

@ArnieGod Ok I got the issue, but wouldn’t this still be dependent on looksrare ensuring a high enough LINK balance? Is there any scenario where this is out of their control (Chainlink fees get unreasonably high)?

Also this issue also doesn’t allow a redraw like forgeries but simply a cancellation after which deposits can be withdrawn. So we have to take it into account before deciding severity, though if all things line up, the winner would indeed lose his winnings.

[ArnieSec]

@nevillehuang if anything I think the chance of this edge case happening here is exponentially higher than trusts report. From what I know the forgeries contract was deployed on 1 chain. This contract will potentially be deployed on 8+ chains as stated in the read me

Mainnet
Arbitrum
Base
Potentially any EVM compatible L2 (Including ZK L2s)

So the issue is at most around 8-10x more likely in this report than in trusts. Given that the team will have to track multiple chainlink balances, the chance this edge case happens is likely. Given that the Impact is severe, loss of funds for users, and breaking core invariants and functionality of the protocol. This was my reasoning for thinking this issue was high severity

[nevillehuang]

@ArnieGod As far as I know, there will be automated bots assigned to ensure sufficient LINK balance in place. This would fall under out of scope admin configurations/conditions that admins are trusted to maintain.

I understand you mentioned a point about the need to not consider this configurations that are out of scope, but based on my understanding of how sherlock will judge this, if an issue is dependent on such out of scope mechanisms, this will likely be invalid. I believe no further discussions is required, I will leave it to @Czar102 to decide

[ArnieSec]

@nevillehuang i admire your ability to see both sides of the argument. I think we just leave it up to the head judge now.

My final point

Discord messages or DM screenshots are not considered sources of truth while judging an issue/escalation especially if they are conflicting with the contest README.

Since there was no mention of the bots in readme, according to Sherlock rules we must judge this issue with the assumption that there is no bots that are in charge with the link funding.

Therefore if we judge this issue fairly and in accordance with Sherlock documentation. I strongly believe this issue is a valid high.

[Czar102]

It is a part of protocol maintenance to top the LINK balance up. Even in an out-of-scope scenario where admins/developers don't top up, any user can top up LINK (essentially like a gas cost) to have randomness fulfilled to their bet.
Even though it would be nice to have a signal of protocol maintenance in the README, it's not required to assume that someone will top up, given the ease of observing the delays in the protocol working as intended.

Planning to reject the escalations and leave the issue as is.

[ArnieSec]

@Czar102

you can calculate the cost of a request on this chainlink link
https://docs.chain.link/vrf/v2/estimating-costs?network=ethereum-mainnet

inputting 500k as the callback gas which is what is used by the team

    uint256 requestId = VRF_COORDINATOR.requestRandomWords({

        keyHash: KEY_HASH,
        subId: SUBSCRIPTION_ID,
        minimumRequestConfirmations: uint16(3),
        callbackGasLimit: uint32(500_000),
        numWords: uint32(1)
    });

we get this from the calculator

Estimated cost per request: 2.078 LINK
Maximum cost per request under the selected gas lane: 15.85 LINK

Link price is currently 18$

thinking its logical that a user will pay 36$ just to fulfill his randomness is actually insane (this is not simply just gas cost, this is a substantial amount of money that i think virtually no one will ever pay). when hes not even guaranteed to win anything. Also the maximum cost per request can be up to 15 LINK, or 270.

it's not required to assume that someone will top up, given the ease of observing the delays in the protocol working as intended.

Assuming a random user will pay 36$ to fulfill a function that guarantees him 0$ in this greed filled space is actually absurd to me, especially when many games on YOLO currently end in the winner profiting 24$ or .01 eth. Its almost certain no user will top up LINK in this scenario...

If you actually think a user will pay up to 270 USD to fulfill his request then I guess this issue is invalid.
what is really more likely, a random user paying up to 270 USD to fulfill a function that has no guarantee he will win any money, or the top up bot running out of funds? let us be logical here... Regardless the sponsor employed a fix so ill be happy knowing no one will lose funds because of this.

[Czar102]

Result:
Invalid
Has duplicates

[sherlock-admin2]

Escalations have been resolved successfully!

Escalation status:

• zzykxx: rejected
• ArnieGod: rejected