enfrasico - Lack of slippage for `heal()` can cause huge financial loss for users

[sherlock-admin2]

enfrasico

high

Lack of slippage for heal() can cause huge financial loss for users

Summary

Users can choose to heal their agent when it is wounded. But the only allowed parameter is the agentId. It does not allow users to specify the maximum amount they are willing to pay for the heal.

Vulnerability Detail

The issue here is that the cost of healing doubles for each heal that has been made. Users will always want to pay a lower cost. Because of the missing slippage check, users can potentially be forced to pay more than 2x the cost they are willing to pay.

Assuming 0.08 USDC LOOKS price ( current price )

1. User sees the cost to heal is 500 LOOKS ( $40 USDC )
2. User calls the function heal
3. 2 other users happen to heal in the same block before the vulnerable user
4. Price now is 2000 LOOKS ( $160 USDC )
5. User pays 4x more than he is willing to

Impact

Due to cost of healing doubles every heal, the cost increases exponentially and can quickly rise beyond what a user is willing to pay for, hence causing huge losses to the user.

Code Snippet

https://github.com/sherlock-audit/2023-10-looksrare/blob/main/contracts-infiltration/contracts/Infiltration.sol#L801

Tool used

Manual Review

Recommendation

Add a maximum amount of LOOKS token in the heal() function, so that users can control and specify how much they are willing to pay for the heal.

Duplicate of #89

[sherlock-admin2]

Escalate

This issue is different from all of its duplicate. The issue highlighted here is targetted at the Infiltration.sol contract. The heal() function in this contract is different from the one in the InfiltrationPeriphery.sol contract.

The sponsor's response of slippage not being needed as ETH is sent in with the call is not applicable here. In the Infiltration.sol contract, heal() calculates the cost of LOOKS paid directly from the current state of the blockchain. This amount is that taken from the user directly. As long as the user has approved an amount higher than the intended cost, this vulnerability WILL occur. This is extremely likely since approving the maximum ERC20 token is commonplace.

This issue should be a valid unique one. In fact, this is a definitive loss of funds in a core function and heal() is pivotal to the infiltration game and will be called multiple times throughout hence I believe this issue to be a high instead of a medium.

You've deleted an escalation for this issue.

[nevillehuang]

Thanks for bringing my attention to this finding. This finding should remain invalid but not a duplicate of #89. Cost to heal calculated here by _costToHeal() in the Infiltration.sol contract is a fixed value not dependent on price since no swap is initiated.

The PoC doesn’t make sense, as the cost calculated is denoted in amount of LOOKS and only dependent on heal count, so users will never incur any slippage.

[Coareal]

heal() cost doubles each time an agent is healed. Please see this line.

Slippage is the difference in expected price and price paid. Because it is possible for multiple heals to happen in the same block, slippage absolutely can happen as I have shown in my report.

[0xllill]

Hello @Coareal , I am the head of judging for this contest. Maybe either you or me didn't understand well the code but from what I see the cost to heal an agent double for one agent each time this agent is healed as the healCount parameter sent to the _costToHeal() comes from the agent struct with a specific agentIds so if I have agent with agentId 500 and agent with agentId 700 is healed before me the cost for healing my agent(500) doesn't change, am I wrong ?

[Coareal]

@nasri136 Yes you are right. I mistook it as a shared global healCount rather than individual agent healCount.