001.md

Joyous Raisin Yak

High

Reentrancy Vulnerability in AmirX.sol Swap and Fee Dispersal Functions

Summary

The lack of a reentrancy guard on the swap and _feeDispersal functions in AmirX.sol may allow malicious actors to reenter and manipulate token swaps or fee calculations, leading to potential fund losses or incorrect fee allocations. Additionally, the ownership checks for accessing critical functions are insufficient, relying on a basic msg.sender == owner check that can be easily targeted.

Root Cause

In AmirX.sol, critical functions such as swap() and internal helpers like _feeDispersal() involve multiple state-changing calls and do not use the nonReentrant modifier or a reentrancy guard. The ownership checks are implemented as require(msg.sender == owner), which lacks the robustness of role-based access controls.

Example: In AmirX.sol lines 112-145, the swap() function performs multiple state-changing operations involving token balances and referrals but lacks a nonReentrant modifier.

Internal pre-conditions

1. Multiple state-changing operations are executed within the same function.
2. No use of nonReentrant modifier in any function that deals with balance updates.
3. Ownership checks rely on a weak require(msg.sender == owner) validation instead of role-based permissions.

External pre-conditions

1. A reentrancy attack is initiated during swap execution.
2. The attacker can call the swap() function multiple times before the initial call completes.
3. Ownership of the contract could be targeted or compromised, leading to unauthorized execution of critical functions.

Attack Path

1. User A starts a swap transaction, which triggers fee dispersal and token allocation.
2. Before the swap completes, User A reenters by calling swap() again, leveraging the pending state changes to manipulate balances.
3. The attacker withdraws tokens beyond their correct share due to manipulated internal state.

Impact

Funds or fees may be manipulated, leading to losses for legitimate users or incorrect accounting in the system. Compromised ownership checks can result in unauthorized execution of swap and fee dispersal operations.

PoC

function swap(address from, uint256 amount) public {
    require(msg.sender == owner, "Only owner"); // Weak ownership check
    
    // Missing nonReentrant modifier
    _updateBalances(from, amount);
    _feeDispersal(amount);
    emit SwapExecuted(from, amount);
}

Mitigation

ntroduce Reentrancy Guard: Add a nonReentrant modifier to the swap() function and any internal helper that involves state changes. Utilize OpenZeppelin's ReentrancyGuard:

import "@openzeppelin/contracts/security/ReentrancyGuard.sol";

contract AmirX is ReentrancyGuard {
    function swap(address from, uint256 amount) public nonReentrant {
        _updateBalances(from, amount);
        _feeDispersal(amount);
        emit SwapExecuted(from, amount);
    }
}

Replace Weak Ownership Check: Implement role-based access control using OpenZeppelin's AccessControl: require(hasRole(SWAPPER_ROLE, msg.sender), "Caller is not authorized to swap");

Test Recommendations:

• Reentrancy Attack Simulation: Implement a reentrancy attack simulation to validate that the updated functions are secure against such vulnerabilities.
• Role Validation Tests: Add test cases to ensure that only accounts with the correct role (SWAPPER_ROLE) can execute swap-related functions.