Japy69 - Unsafe use of tx.origin in the open function will lead to unauthorized liquidation

[sherlock-admin3]

Japy69

Medium

Unsafe use of tx.origin in the open function will lead to unauthorized liquidation

Summary

The use of tx.origin in the liquidate function of the Velar protocol exposes users to phishing attacks. If a user grants unlimited allowance to the contract for base_token or quote_token, a malicious contract can execute the liquidate function on their behalf, resulting in unauthorized liquidation of positions. The root cause is the use of tx.origin to determine the liquidator, which is unsafe as it can be manipulated by intermediate contracts.

Root Cause

In core.vy:318, there is an unsafe use of tx.origin, which can lead to unauthorized liquidation of positions. The choice to use tx.origin might be due to the API.vy contract serving as an intermediary between the user and the core.vy contract. However, tx.origin is considered insecure because it references the original sender of a transaction, which can lead to unintended consequences when intermediary contracts are involved. The Solidity documentation provides a detailed explanation of why tx.origin is unsafe: Solidity Security Considerations - tx.origin.

The liquidate function in core.vy allows users to liquidate a position by calling the function and transferring the liquidation fee to the liquidator. The function uses tx.origin to identify the user initiating the transaction:

user : address = tx.origin  # liquidator

This design is problematic because tx.origin represents the original sender of the transaction, not necessarily the direct caller. In scenarios where a user interacts with another contract that in turn calls the liquidate function, tx.origin will still point to the original user.

Internal Pre-conditions

1. The victim must have granted an allowance to the core contract address for either base_token or quote_token, enabling the contract to transfer tokens on the victim's behalf.

2. The victim must initiate a transaction with a smart contract that allows the attacker to indirectly call the liquidate function at any point during the transaction. This can be easily achieved, for example, if the victim interacts with an automated router like Uniswap's that swaps tokens based on optimal trade routes. The attacker could create a malicious token involved in the trade, and within the transfer function of this malicious token, the attacker can execute a call to the Velor protocol's liquidate function. Because tx.origin will still refer to the victim, the Velor protocol will perceive the transaction as initiated by the victim.

External Pre-conditions

No response

Attack Path

If a user has set an unlimited allowance for base_token or quote_token to this contract, a malicious contract can execute a phishing attack by:

1. Encouraging the user to initiate a transaction on the malicious contract. As explained in internal pre-conditions, it is not hard.
2. Having the malicious contract call the liquidate function on API.vy.
3. Using tx.origin to transfer tokens from the user’s address to the contract, liquidating a position without the user's explicit consent.

Impact

• Unauthorized Position Liquidation: An attacker can exploit this vulnerability to force a user to liquidate positions, leading to unauthorized token movements and potential financial losses due to unexpected liquidation conditions.

PoC

No response

Mitigation

To mitigate this vulnerability, replace tx.origin with a user parameter passed to the liquidate function. This parameter should be set by the msg.sender when the API.vy contract calls the function. The updated function signature should look like this:

@external
def liquidate(
  user        : address,
  id          : uint256,
  base_token  : address,
  quote_token : address,
  position_id : uint256,
  ctx         : Ctx) -> PositionValue:

Additionally, the contract call on api.vy:211 should be updated to:

  return self.CORE.liquidate(msg.sender, 1, base_token, quote_token, position_id, ctx)

By passing msg.sender as the user, the function ensures that only the immediate caller is authorized to initiate the liquidation of a position, thus preventing phishing attacks.

Duplicate of #82