sherlock-audit / 2024-02-jala-swap-judging

6 stars 4 forks source link

MatricksDeCoder - Protocol may not work with upgradeable tokens #204

Closed sherlock-admin closed 7 months ago

sherlock-admin commented 8 months ago

MatricksDeCoder

medium

Protocol may not work with upgradeable tokens

Summary

Upgradeable ERC20 tokens may open up attack vectors. They may appear inert initially

Vulnerability Detail

However when upgraded they can become ERC777 or become fee on transfer or activate a fee, become rebasing, flash mintable, enact multiple addresses etc after upgrade leading to potential incompatibility with router and opening various attack vectors

Impact

Liquidity pool can be stolen in some ERC777 tokens when expectation was not callback tokens Router may be incompatible with token with fees on transfer

Code Snippet

function addLiquidity(
        address tokenA,
        address tokenB,
        uint256 amountADesired,
        uint256 amountBDesired,
        uint256 amountAMin,
        uint256 amountBMin,
        address to,
        uint256 deadline
    ) external virtual override ensure(deadline) returns (uint256 amountA, uint256 amountB, uint256 liquidity) {
        (amountA, amountB) = _addLiquidity(tokenA, tokenB, amountADesired, amountBDesired, amountAMin, amountBMin);
        address pair = JalaLibrary.pairFor(factory, tokenA, tokenB);
        TransferHelper.safeTransferFrom(tokenA, msg.sender, pair, amountA);
        TransferHelper.safeTransferFrom(tokenB, msg.sender, pair, amountB);
        liquidity = IJalaPair(pair).mint(to);
    }
function swapExactTokensForTokens(
        uint256 amountIn,
        uint256 amountOutMin,
        address[] calldata path,
        address to,
        uint256 deadline
    ) external virtual override ensure(deadline) returns (uint256[] memory amounts) {
        amounts = JalaLibrary.getAmountsOut(factory, amountIn, path);
        if (amounts[amounts.length - 1] < amountOutMin) revert JalaLibrary.InsufficientOutputAmount();
        TransferHelper.safeTransferFrom(
            path[0],
            msg.sender,
            JalaLibrary.pairFor(factory, path[0], path[1]),
            amounts[0]
        );
        _swap(amounts, path, to);
    }

Tool used

Manual Review

Recommendation

May be recommended to whitelist tokens and avoid upgradeable tokens May be recommended to monitor upgradeable tokens and preempt or proactive or react to changes that may introduce fees, callbacks etc Consider introducing logic that will freeze interactions with the token in question if an upgrade is detected.

Duplicate of #117