Swapper Role will lose funds due to flowed logic

Summary

during _buyBack in _feeDispersal in _defiSwap the swapper role is forwarding msg.value although it should be the balance of the contract, leading to loss of funds to swapper role or txn reverts

Root Cause

doing msg.value instead of this.balance

Internal pre-conditions

There is fees need to be dispersed

External pre-conditions

None

Attack Path

User doing a defiswap

Fees paid are paid by the swapper role not the funds of user AmirX

Remainer of funds are sent to safe

Leading to swapper role paying for defiswaps fees which is not intended

Impact

loss of funds for swapper role

PoC

in Line 233 is the mistake

File: AmirX.sol
225:     function _buyBack(
226:         ERC20 feeToken,
227:         address aggregator,
228:         address safe,
229:         bytes memory swapData
230:     ) internal {
231:         if (address(feeToken) == address(0)) return;
232:         if (address(feeToken) == POL) {
233:             (bool polSwap, ) = aggregator.call{value: msg.value}(swapData);
234:             require(polSwap, "AmirX: POL swap transaction failed");
235: 
236:             if (address(this).balance > 0) {
237:                 (bool success, ) = safe.call{value: address(this).balance}("");
238:                 require(success, "AmirX: POL send transaction failed");
239:             }
240:         } else {
241:             // zero out approval
242:             feeToken.forceApprove(aggregator, 0);
243:             feeToken.safeIncreaseAllowance(
244:                 aggregator,
245:                 feeToken.balanceOf(address(this))
246:             );
247: 
248:             (bool ercSwap, ) = aggregator.call{value: 0}(swapData);
249:             require(ercSwap, "AmirX: token swap transaction failed");
250: 
251:             uint256 remainder = feeToken.balanceOf(address(this));
252:             if (remainder > 0) feeToken.safeTransfer(safe, remainder);
253:         }
254:     }

Mitigation

do address(this).balance

sherlock-audit / 2024-11-telcoin-judging

Hunter - Swapper Role will lose funds due to flowed logic #186