code-423n4/2024-07-loopfi-validation

Incorrect Updating of `maxAmountsIn` for Tokens After Pool Token

Closed this issue · 0 comments

Lines of code

https://github.com/code-423n4/2024-07-loopfi/blob/57871f64bdea450c1f04c9a53dc1a78223719164/src/proxy/PoolAction.sol#L181

Vulnerability details

Vulnerability Details

The updateLeverJoin function in the PoolAction contract updates the maxAmountsIn array for joining a Balancer pool with leveraged positions. It should correctly update this array for all tokens, including those that come after the pool token (BPT) in the array.

The function's current implementation may lead to incorrect updates of the maxAmountsIn array for tokens positioned after the pool token (BPT) in the assets array. This is due to the misalignment caused by the skipIndex logic, which affects the indexing for all tokens after the BPT.

Code Snippet

bool skipIndex = false;
for (uint256 i = 0; i < len; ) {
    uint256 assetIndex = i - (skipIndex ? 1 : 0);
    if (assets[i] == joinToken) {
        maxAmountsIn[i] = joinAmount;
        assetsIn[assetIndex] = joinAmount;
    } else if (assets[i] == upFrontToken && assets[i] != poolToken) {
        maxAmountsIn[i] = upfrontAmount;
        assetsIn[assetIndex] = upfrontAmount;
    } else {
        skipIndex = skipIndex || assets[i] == poolToken;
    }
    unchecked {
        ++i;
    }
}

Impact

  1. Tokens after the BPT in the assets array may have their maxAmountsIn values incorrectly set or left unchanged.
  2. This can lead to joins with incorrect maximum input amounts, potentially causing failed transactions or unintended token transfers.
  3. In severe cases, it could result in economic loss due to incorrect pool share calculations.

Scenario

Consider a Balancer pool with tokens [TokenA, BPT, TokenB, TokenC], where TokenC is the joinToken:

  • TokenA: Correctly updated
  • BPT: Skipped (correct behavior)
  • TokenB: May be incorrectly updated or skipped
  • TokenC: May have its maxAmountsIn value set to an incorrect index in the array

Fix

Modify the loop to correctly handle all tokens, regardless of their position relative to the BPT:

uint256 assetsInIndex = 0;
for (uint256 i = 0; i < assets.length; i++) {
    if (assets[i] == poolToken) {
        maxAmountsIn[i] = 0;  // Explicitly set to 0 for pool token
    } else {
        if (assets[i] == joinToken) {
            maxAmountsIn[i] = joinAmount;
            assetsIn[assetsInIndex] = joinAmount;
        } else if (assets[i] == upFrontToken) {
            maxAmountsIn[i] = upfrontAmount;
            assetsIn[assetsInIndex] = upfrontAmount;
        }
        assetsInIndex++;
    }
}

Assessed type

Context