036.md

Bitter Rouge Alpaca

High

Incorrect Numa fee logic

Summary

Incorrect fee checks on internal _transfer() cause users to pay fee, when they are expected to not.

Root Cause

The Numa.sol has two fee related mappings; https://github.com/sherlock-audit/2024-12-numa-audit/blob/ae1d7781efb4cb2c3a40c642887ddadeecabb97d/Numa/contracts/NumaStore.sol#L5

    struct NumaStorage {
        uint sellFeeBips;
        mapping(address => bool) isIncludedInFees;
        mapping(address => bool) wlSpenders;
    }

if wlSpenders[addr]==true || isIncludedInFees[addr]==false, no fee is supposed to be charge to the addr.

From the scripts/NumaToken/updateFee.js and code comments, https://github.com/sherlock-audit/2024-12-numa-audit/blob/main/Numa/scripts/NumaToken/updateFee.js https://github.com/sherlock-audit/2024-12-numa-audit/blob/ae1d7781efb4cb2c3a40c642887ddadeecabb97d/Numa/contracts/Numa.sol#L94

, we can say, no fee will charge if spender is router(or wlSpenders[router]==true).

Consider a user addLiquidity() via router,

1. Router calls transferFrom on numa token, since wlSpenders[router]==true, the fee get bypass and proceed to internal _transfer() call

2. The internal ERC20 _transfer() function is override by numa, and since isIncludedInFees[pair]==true, it charges fee when transferring it to pair from router here

    function _transfer(
        address from,
        address to,
        uint256 amount
    ) internal virtual override {
        // uniswap sell fee
        NumaStorage storage ns = numaStorage();
        uint fee = ns.sellFeeBips;
        // apply (burn) fee on some receivers. Typically, the UniswapV2Pair, to apply fee when selling on Uniswap.
        if ((fee > 0) && ns.isIncludedInFees[to]) {
            _transferWithFee(from, to, amount, fee);
        } else {
            super._transfer(from, to, amount);
        }
    }

Note that the _transfer() here is expected to charge fee(see above comment), in case users try to sell/swap luma for any other token on uniswap. However, the case where users provides liquidity, the logic above also charges fee to users providing liquidity in numa tokens, where its expected to not(as can be inferred from the comment as well).

https://github.com/sherlock-audit/2024-12-numa-audit/blob/ae1d7781efb4cb2c3a40c642887ddadeecabb97d/Numa/contracts/Numa.sol#L94

// cancel fee for some spenders. Typically, this will be used for UniswapV2Router which is used when adding liquidity

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

No response

Impact

Users end up paying extra in fees when they are expected to not.

PoC

No response

Mitigation

Override the transfer() function instead of internal _transfer() to charge fee on selling of numa tokens only,