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

[sherlock-admin4]

Japy69

Medium

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

Summary

The use of tx.origin in the open 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 open function on their behalf, resulting in unauthorized creation of leveraged positions. The root cause is the use of tx.origin to determine the user, which is unsafe as it can be manipulated by intermediate contracts.

Root Cause

In core.vy:241, there is an unsafe use of tx.origin, which can lead to unauthorized opening of leveraged 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 open function in core.vy allows users to open leveraged positions by depositing collateral and leveraging it against the pool. The function uses tx.origin to identify the user initiating the transaction:

user : address = tx.origin

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 open 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 open 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 open 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 open function on API.vy.
3. Using tx.origin to transfer tokens from the user’s address to the contract, opening a leveraged position without the user's explicit consent.

Impact

• Unauthorized Position Opening: An attacker can exploit this vulnerability to force a user to open leveraged positions, leading to unauthorized token movements and potential financial losses due to unexpected margin calls or liquidations.

PoC

No response

Mitigation

To mitigate this vulnerability, replace tx.origin with a user parameter passed to the open 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 open(
  user        : address,
  id          : uint256,
  base_token  : address,
  quote_token : address,
  long        : bool,
  collateral0 : uint256,
  leverage    : uint256,
  ctx         : Ctx) -> PositionState:

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

  return self.CORE.open(msg.sender, 1, base_token, quote_token, long, collateral0, leverage, ctx)

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

Duplicate of #82