sherlock-audit / 2024-01-looksrare-judging

3 stars 0 forks source link

franfran20 - VRF ` Min Request Confirmations` Fixed At 3 in contract which is < Depth Of Chain ReOrgs For Chain To Be Deployed On #147

Closed sherlock-admin closed 7 months ago

sherlock-admin commented 7 months ago

franfran20

medium

VRF Min Request Confirmations Fixed At 3 in contract which is < Depth Of Chain ReOrgs For Chain To Be Deployed On

Summary

When requesting randomness in the _drawWinner function, the minimumRequestConfirmations parameter is set to 3. This value must be greater than the depth of common chain re-organizations on the chains the YoloV2 is to be deployed, as chain re-organizations re-order blocks & transactions, which can affect the returned randomness.

Vulnerability Detail

This could result in a winner becoming a loser or vice-versa due to the chain re-organization re-ordering the randomness request, resulting in a different randomness result. This is less than the common chain re-org values on polygon. You can check out the common re-orgs depth for polygon POS under re-orgs depth and see how frequent its greater than 3 here.

How do i know YoloV2 would be deployed on polygon POS. Here is a discord message from one of the team members validating that if you actually find an issue that is specific to Polygon, then it should be a valid issue here

Impact

Medium This could result in a winner becoming a loser or vice-versa due to the chain re-organization re-ordering the randomness request, resulting in a different randomness result.

Code Snippet

Link to the snippet here

Tool used

Manual Review

Recommendation

Increase the minimumRequestConfirmations from 3 or include its as a constructor parameter on deployment.

Duplicate of #41