Description

The function containTokenset works by looping over a set of tokens (the parameter b) to ensure a larger token set (the parameter a) contains all the tokens in b and contains more than the amount in b:

function containTokenset(Token[] memory a, Token[] memory b) internal pure returns (bool) {
    uint k;
    for (uint i = 0; i < b.length; i++) {
        k = a.length;
        for (uint j = 0; j < a.length; j++) {
            if (isSameToken(b[i], a[j])) {
                if (a[j].amount < b[i].amount) {
                    return false;
                }
                k = j;
                break;
            }
        }
        if (k == a.length) {
            return false;
        }
    }
    return true;
}

Therefore, the given small token set must not have any duplication, or the returned result may be incorrect. For example, if the large token set contains a token with an amount of 10, and the small token set contains two identical tokens, each with an amount of 6, the containTokenset function will return true even though the large set does not contain the amount of the smaller set.

Impact

The function containTokenset does not perform a duplicate check on the given token set. This function is called by addRebalanceRequest, which computes a new token basket set and finally calls hasDuplicates to verify if it contains any duplicate tokens. However, the function addBurnFeeRequest receives an order and computes the fees associated to the sellTokenset. Then, the function checks if the asset has enough tokens to pay the fees by calling the containTokenset function, but it does not check for any duplicates. The resulting computed fees may then be larger than the asset fees.

Recommendations

We recommend implementing a correct duplicate check that could be integrated to containTokenset.

Remediation

This issue has been acknowledged by SoSoValue, and a fix was implemented in commit 7ddd9c5d↗.