IndexPool Manual Review Findings

IPL-01M: Insecure Proportion Calculation

Type	Severity	Location
Logical Fault		IndexPool.sol:L47, L63

Description:

The IndexPool::burnIndexTokenAndSkipSomeTokens function is presently incorrect as it will burn the tokens of the user prior to measuring the total supply of the token, thereby skewing the actual proportion of the user's balance.

Impact:

It is presently possible to capture the full available balance of the IndexPool with half the total supply of the token, and the code will consistently overvalue all burn operations running out of funds before it is meant to.

Example:

Copy
42function burnIndexTokenAndSkipSomeTokens(
43    uint256 _amount,
44    address[] memory _skipTokens
45) public nonReentrant {
46    uint256 amountOfPoolBalance;
47    indexToken.burnFrom(msg.sender, _amount);
48    for (uint8 i = 0; i < heldTokens.length; i++) {
49        // Skip tokens that are in the skipTokens array
50        bool skipToken = false;
51        for (uint8 j = 0; j < _skipTokens.length; j++) {
52            if (heldTokens[i] == _skipTokens[j]) {
53                skipToken = true;
54                break;
55            }
56        }
57        if (skipToken) {
58            continue;
59        }
60
61        amountOfPoolBalance =
62            ((IERC20(heldTokens[i])).balanceOf(address(this)) * _amount) /
63            indexToken.totalSupply();
64        if (amountOfPoolBalance > 0) {
65            (IERC20(heldTokens[i])).safeTransfer(
66                msg.sender,
67                amountOfPoolBalance
68            );
69        }
70    }
71    emit IndexTokenBurned(msg.sender, _amount);
72}

Recommendation:

We advise the total supply to be snapshotted prior to the burning operation, ensuring that the percentage calculations within the for loop of the IndexPool::burnIndexTokenAndSkipSomeTokens function are correct.

Alleviation (d639d227f8):

The system was updated to perform the burn operation after the tokens have been distributed which might be insecure in case re-entrant tokens are introduced as the user would be able to liquidate the captured funds to purchase the underlying indexToken that correspond to them.

We advise our original recommendation to be applied, burning the relevant tokens right after measuring the total supply.

Alleviation (2ef12e6885):

The burn operation was re-ordered as advised, ensuring it is executed right after the total supply snapshot and thus preventing any reward distribution re-entrancies to affect the contract's operation.