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

Cayde-6 - wrong value being passed in _buyBack lead to loss of funds for swapper #221

Open
sherlock-admin4 opened this issue Nov 13, 2024 · 1 comment
Labels
Sponsor Confirmed The sponsor acknowledged this issue is valid Will Fix The sponsor confirmed this issue will be fixed

Comments

@sherlock-admin4
Copy link

sherlock-admin4 commented Nov 13, 2024

Cayde-6

Medium

wrong value being passed in _buyBack lead to loss of funds for swapper

Summary

An incorrect value is being passed in the _buyBack function within the _feeDispersal of _defiSwap. The SWAPPER_ROLE is forwarding msg.value instead of the contract’s balance, leading to unintended fund losses for the swapper role.

Root Cause

The function mistakenly forwards msg.value instead of address(this).balance.

Internal Pre-Conditions

Executing a defi swap with fee dispersal enabled.

Attack Path

  1. A user initiates a defi swap.
  2. The fees for the swap are paid from the funds of the swapper role, rather than being deducted from the user's funds (e.g., AmirX).
  3. The remaining funds are sent to safe.
  4. As a result, the swapper role ends up covering the fees for defi swaps, which is unintended.

Impact

The SWAPPER_ROLE incurs a loss of funds due to covering defi swap fees.

Proof of Concept (PoC)

AmirX.sol#L232

 function _buyBack(
     ERC20 feeToken,
     address aggregator,
     address safe,
     bytes memory swapData
 ) internal {
     if (address(feeToken) == address(0)) return;
     if (address(feeToken) == POL) {
@>       (bool polSwap, ) = aggregator.call{value: msg.value}(swapData);
         require(polSwap, "AmirX: POL swap transaction failed");

Mitigation

To resolve this, replace msg.value with address(this).balance to ensure the contract's balance is used instead of the caller’s sent value.

@sherlock-admin2
Copy link
Contributor

The protocol team fixed this issue in the following PRs/commits:
https://github.com/telcoin/telcoin-audit/pull/59

@sherlock-admin3 sherlock-admin3 added Sponsor Confirmed The sponsor acknowledged this issue is valid Will Fix The sponsor confirmed this issue will be fixed labels Nov 15, 2024
@sherlock-admin3 sherlock-admin3 changed the title Winning Viridian Crocodile - wrong value being passed in _buyBack lead to loss of funds for swapper Cayde-6 - wrong value being passed in _buyBack lead to loss of funds for swapper Nov 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Sponsor Confirmed The sponsor acknowledged this issue is valid Will Fix The sponsor confirmed this issue will be fixed
Projects
None yet
Development

No branches or pull requests

3 participants