Issue 1

[Edward-Lo]

Description

FlamingoSwapRouter contract provides interfaces for users to add/remove liquidity and swap tokens. The function AddLiquidity can add tokens to the pools and mint liquidity tokens for the user.

public static BigInteger[] AddLiquidity(byte[] sender, byte[] tokenA, byte[] tokenB, BigInteger amountADesired, BigInteger amountBDesired, BigInteger amountAMin, BigInteger amountBMin, BigInteger deadLine)
{
    Assert(Runtime.CheckWitness(sender), "Forbidden");

    Assert((BigInteger) Runtime.Time <= deadLine, "Exceeded the deadline");


    var reserves = GetReserves(tokenA, tokenB);
    var reserveA = reserves[0];
    var reserveB = reserves[1];
    BigInteger amountA = 0;
    BigInteger amountB = 0;
    if (reserveA == 0 && reserveB == 0)
    {
        amountA = amountADesired;
        amountB = amountBDesired;
    }
    else
    {
        var estimatedB = Quote(amountADesired, reserveA, reserveB);
        if (estimatedB <= amountBDesired)
        {
            Assert(estimatedB >= amountBMin, "Insufficient B Amount");
            amountA = amountADesired;
            amountB = estimatedB;
        }
        else
        {
            var estimatedA = Quote(amountBDesired, reserveB, reserveA);
            Assert(estimatedA >= amountAMin, "Insufficient A Amount"); 
            amountA = estimatedA;
            amountB = amountBDesired;
        }
    }

    var pairContract = GetExchangePairWithAssert(tokenA, tokenB);

    SafeTransfer(tokenA, sender, pairContract, amountA);
    SafeTransfer(tokenB, sender, pairContract, amountB);

    var liquidity = pairContract.DynamicMint(sender);//+0.03gas
    //var liquidity = ((Func<string, object[], BigInteger>)pairContract.ToDelegate())("mint", new object[] { sender });
    return new BigInteger[] { amountA.ToBigInt(), amountB.ToBigInt(), liquidity };
}

The parameters amountADesired and amountBDesired are the maximum tokens that can be transferred in. The final input token amounts amountA and amountB should be less than amountADesired and amountBDesired.

However, in the case estimatedB is bigger than amountBDesired. The contract misses a check that estimatedA should also be less than amountADesired.

Recommendation

public static BigInteger[] AddLiquidity(byte[] sender, byte[] tokenA, byte[] tokenB, BigInteger amountADesired, BigInteger amountBDesired, BigInteger amountAMin, BigInteger amountBMin, BigInteger deadLine)
{
    Assert(Runtime.CheckWitness(sender), "Forbidden");

    Assert((BigInteger) Runtime.Time <= deadLine, "Exceeded the deadline");


    var reserves = GetReserves(tokenA, tokenB);
    var reserveA = reserves[0];
    var reserveB = reserves[1];
    BigInteger amountA = 0;
    BigInteger amountB = 0;
    if (reserveA == 0 && reserveB == 0)
    {
        amountA = amountADesired;
        amountB = amountBDesired;
    }
    else
    {
        var estimatedB = Quote(amountADesired, reserveA, reserveB);
        if (estimatedB <= amountBDesired)
        {
            Assert(estimatedB >= amountBMin, "Insufficient B Amount");
            amountA = amountADesired;
            amountB = estimatedB;
        }
        else
        {
            var estimatedA = Quote(amountBDesired, reserveB, reserveA);
            Assert(estimatedA <= amountADesired, "Insufficient A Amount"); 
            Assert(estimatedA >= amountAMin, "Insufficient A Amount"); 
            amountA = estimatedA;
            amountB = amountBDesired;
        }
    }

    var pairContract = GetExchangePairWithAssert(tokenA, tokenB);

    SafeTransfer(tokenA, sender, pairContract, amountA);
    SafeTransfer(tokenB, sender, pairContract, amountB);

    var liquidity = pairContract.DynamicMint(sender);//+0.03gas
    //var liquidity = ((Func<string, object[], BigInteger>)pairContract.ToDelegate())("mint", new object[] { sender });
    return new BigInteger[] { amountA.ToBigInt(), amountB.ToBigInt(), liquidity };
}

[Ashuaidehao]

It's just a mathematic problem. Check the formula in "Quote" function, if use amountADesired calculating estimatedB bigger than amountBDesired, in this case, use amountBDesired calculating estimatedA will never greater than amountADesired.
I'll add Assert(estimatedA <= amountADesired, "Insufficient A Amount"); for more comprehensible, thanks for your suggestion.

[heyJonBray]

No math should ever be done when it comes to assets without assertions first. I do not know where to start with the issues as I am independently auditing this repo. The handful of issues in place indicate a complete lack of understanding in the basic framework on which this is built.

I came here to look over the contracts, but there are so many issues I am not quite sure even where to begin. Both contracts are ripe with problems. The math library from C# can be bypassed due to a perceived overload from the NEO libraries that can be called from the browser; timestamps are not implemented, leaving the base libraries open for time-based attacks; and the wrappers do not correctly correlate to prices, leaving the potential for whales to swallow up one half of a pool due to extreme arbitrage opportunities.

This project, with a team of a dozen developers, should not be released for at least 6 more months... I feel sorry that it is being rushed as it could have been a viable DEX; but is in no way prepared to be a "full stack platform".