Description

In SwapExternal, there is a check that to != pairAddress:

private void SwapInternal(string symbolIn, string symbolOut, long amountIn, long amountOut, Address to,
    string channel)
{
    // [...]

    Assert(to != pairAddress, "Invalid account address");

The purpose of this check in the Uniswap code that this function was adapted from is to ensure that the address that is receiving the symbolOut tokens is not the same as the currently executing contract. This is important because the logic outside the SwapExternal call already transferred the tokens to the target, whereas the math inside it assumes that the entire balance above the reserves is part of the swap. So, if the intended destination for the tokens is the pool itself, the destination tokens will be double-counted.

However, in this code, no matter what the pair is, if it is an internal swap pair, to will always be the Context.Self address. This is because the virtual addresses do not hold the actual tokens — accounting for token ownership happens internally through State.PoolBalanceMap only.

Impact

If a swap path contains the same pair twice, this check fails to prevent the destination symbol balance from being double-counted. For example, if the SwapExactTokensForTokens in the Case0Test in AwakenSwapContractTests is replaced with Path = {"ELF","TEST","ELF"}, then this assert fails to detect the problem.

There is no actual impact, since the later check Error with K catches the issue — if this check was not there, then this would be a critical issue, because value would be lost through a decreased K.

With careful manipulation of pairs and parameters, it may be possible to bypass fees using this exploit by swapping pairs in a way that this exploit covers swap fees without decreasing K.

Recommendations

Instead of comparing to with pairAddress, this check should compare nextPair with it. This check can be more easily implemented outside SwapInternal.

Remediation

This issue has been acknowledged and fixed in the following commits: