Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Japy69 - Unsafe use of tx.origin in the mint function will leading to unauthorized LP token minting #1

Closed
sherlock-admin2 opened this issue Sep 9, 2024 · 2 comments
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Medium A Medium severity issue. Reward A payout will be made for this issue Sponsor Disputed The sponsor disputed this issue's validity

Comments

@sherlock-admin2
Copy link
Contributor

sherlock-admin2 commented Sep 9, 2024

Japy69

Medium

Unsafe use of tx.origin in the mint function will leading to unauthorized LP token minting

Summary

The use of tx.origin in the mint function of the Velar protocol allows for a phishing attack. If a user grants unlimited allowance to the contract for base_token and quote_token, a malicious contract can execute the mint function on their behalf, resulting in unauthorized minting of LP tokens. 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:166 there is an unsafe use of tx.origin as it can cause unauthorized minting.
The choice to use tx.origin may be because of api.vy contract between user and core.vy contract. There is a good explaination of why it is unsafe in the Solidity documentation: https://docs.soliditylang.org/en/latest/security-considerations.html#tx-origin.

The mint function in API.vy allows users to mint LP tokens by depositing base_token and quote_token into the contract. 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 mint 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 both base_token and 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 mint 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 mint 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 and 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 mint function on API.vy.
  3. Using tx.origin to transfer tokens from the user’s address to the contract, minting new LP tokens without the user's explicit consent.

Impact

  • Unauthorized Token Minting: An attacker can exploit this vulnerability to force a user to mint LP tokens, leading to unauthorized token movements, and potential loss of funds.

PoC

No response

Mitigation

To mitigate this vulnerability, replace tx.origin with a user parameter passed to the mint 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 mint(
  user        : address,
  id          : uint256,
  base_token  : address,
  quote_token : address,
  lp_token    : address,
  base_amt    : uint256,
  quote_amt   : uint256,
  ctx         : Ctx) -> uint256:

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

  return self.CORE.mint(msg.sender, 1, base_token, quote_token, lp_token, base_amt, quote_amt, ctx)

By passing msg.sender as the user, the function ensures that only the immediate caller is authorized to initiate the minting process, thus preventing phishing attacks.

Duplicate of #82

@github-actions github-actions bot added Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Medium A Medium severity issue. labels Sep 11, 2024
@sherlock-admin3 sherlock-admin3 changed the title Shaggy Smoke Mole - Unsafe use of tx.origin in the mint function will leading to unauthorized LP token minting Japy69 - Unsafe use of tx.origin in the mint function will leading to unauthorized LP token minting Sep 11, 2024
@sherlock-admin3 sherlock-admin3 added Reward A payout will be made for this issue Sponsor Disputed The sponsor disputed this issue's validity labels Sep 11, 2024
@Coriolan-dev
Copy link

Escalate

I don't see any rules in the readme that make this issue non validated by the sponsor.

@sherlock-admin3
Copy link
Contributor

Escalate

I don't see any rules in the readme that make this issue non validated by the sponsor.

The escalation could not be created because you are not exceeding the escalation threshold.

You can view the required number of additional valid issues/judging contest payouts in your Profile page,
in the Sherlock webapp.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Medium A Medium severity issue. Reward A payout will be made for this issue Sponsor Disputed The sponsor disputed this issue's validity
Projects
None yet
Development

No branches or pull requests

3 participants