Fairness of Randomness is threatened and possibilities for gaming the jackpot.
Summary
Re-requesting randomness from VRF is a security pattern anti-pattern. This issue will show how the current implementation of startGame() goes directly against Chainlink's security Security Consideration of not re-requesting randomness (https://docs.chain.link/vrf/v2/security#do-not-re-request-randomness).
Vulnerability Detail
The fulfillRandomWords function calls internal functions _healRequestFulfilled , _woundRequestFulfilled, and _killWoundedAgents, which loops over the healingAgentIdsCount, woundedAgentsCount and woundedAgentIdsCount. This is a problem as the code might require a lot of gas and make the fulfillRandomWords revert it's execution, which is problem for the integration of VRF as listed in the docs, since it should not be reverting.
Code Snippet
We can see that from the startGame function, _requestForRandomness is can be re ran in the case of a revert.
Re-requesting randomness is achieved when there is a possible revert on the fulfillRandomWords function. Fairness of protocol is undermined.
Tool used
Manual Review
Recommendation
Consider caching the randomness received and then letting an EOA to make the actual call the internal functions listed above, it can also set the correct gas amount.
syahirAmali
medium
Fairness of Randomness is threatened and possibilities for gaming the jackpot.
Summary
Re-requesting randomness from VRF is a security pattern anti-pattern. This issue will show how the current implementation of
startGame()goes directly against Chainlink's security Security Consideration of not re-requesting randomness (https://docs.chain.link/vrf/v2/security#do-not-re-request-randomness).Vulnerability Detail
The
fulfillRandomWordsfunction calls internal functions_healRequestFulfilled,_woundRequestFulfilled, and_killWoundedAgents, which loops over thehealingAgentIdsCount,woundedAgentsCountandwoundedAgentIdsCount. This is a problem as the code might require a lot of gas and make thefulfillRandomWordsrevert it's execution, which is problem for the integration of VRF as listed in the docs, since it should not be reverting.Code Snippet
We can see that from the
startGamefunction,_requestForRandomnessis can be re ran in the case of a revert.https://github.com/sherlock-audit/2023-10-looksrare/blob/main/contracts-infiltration/contracts/Infiltration.sol#L522
Impact
Re-requesting randomness is achieved when there is a possible revert on the
fulfillRandomWordsfunction. Fairness of protocol is undermined.Tool used
Manual Review
Recommendation
Duplicate of #64