sherlock-audit / 2023-06-tokemak-judging

10 stars 8 forks source link

kutugu - Some curve pool oracle prices can be manipulated #718

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

kutugu

high

Some curve pool oracle prices can be manipulated

Summary

This problem is still a well-known readability problem of curve. The reentrant problem cannot be completely avoided by withdraw_admin_fees detection, because for some pools the withdraw_admin_fees function does not have reentrant protection and cannot detect the reentrant state.

Vulnerability Detail

@external
def withdraw_admin_fees():
    receiver: address = Factory(self.factory).get_fee_receiver(self)

    amount: uint256 = self.admin_balances[0]
    if amount != 0:
        raw_call(receiver, b"", value=amount)

    amount = self.admin_balances[1]
    if amount != 0:
        assert ERC20(self.coins[1]).transfer(receiver, amount, default_return_value=True)

    self.admin_balances = empty(uint256[N_COINS])

This section uses stETH/ETH ng and stETH/ETH concentrated as an example, there is no revert in the reentrant state. The attacker can reenter the oracle in the protocol when remove_liquidity transfers ETH. At this time, the get_virtual_price has not changed, and the attacker can use the previous price to carry out arbitrage attacks.

Impact

For some curve pool, oracle prices can be manipulated by reentrant attack.

Code Snippet

Tool used

Manual Review

Recommendation

For special pools, other functions should be used to detect reentrant, such as remove_liquidity

sherlock-admin2 commented 1 year ago

1 comment(s) were left on this issue during the judging contest.

Trumpero commented:

invalid, the contract executes an external call to curve pool's owner not to the curve pool. Since the "stETH/ETH ng" doesn't have the public owner() function, the call will revert before execute the reentrancy check. So I classify this report as invalid due incorrect desciption