Using incorrect function to determine the token supply in a Balancer weighted pool

Summary

The protocol uses the totalSupply() function to obtain the number of LP tokens in a Balancer weighted pool. However, this function does not always return the correct count, and the Balancer documentation recommends using getActualSupply() instead.

Vulnerability Detail

    /**
     * @notice Returns the effective BPT supply.
     *
     * @dev This would be the same as `totalSupply` however the Pool owes debt to the Protocol in the form of unminted
     * BPT, which will be minted immediately before the next join or exit. We need to take these into account since,
     * even if they don't yet exist, they will effectively be included in any Pool operation that involves BPT.
     *
     * In the vast majority of cases, this function should be used instead of `totalSupply()`.
     *
     * **IMPORTANT NOTE**: calling this function within a Vault context (i.e. in the middle of a join or an exit) is
     * potentially unsafe, since the returned value is manipulable. It is up to the caller to ensure safety.
     *
     * This is because this function calculates the invariant, which requires the state of the pool to be in sync
     * with the state of the Vault. That condition may not be true in the middle of a join or an exit.
     *
     * To call this function safely, attempt to trigger the reentrancy guard in the Vault by calling a non-reentrant
     * function before calling `getActualSupply`. That will make the transaction revert in an unsafe context.
     * (See `whenNotInVaultContext` in `WeightedPool`).
     *
     * See https://forum.balancer.fi/t/reentrancy-vulnerability-scope-expanded/4345 for reference.
     */
    function getActualSupply() external view returns (uint256) {
        uint256 supply = totalSupply();

        (uint256 protocolFeesToBeMinted, ) = _getPreJoinExitProtocolFees(
            getInvariant(),
            _getNormalizedWeights(),
            supply
        );

        return supply.add(protocolFeesToBeMinted);
    }

From the comment for the getActualSupply() function in the Balancer source code, it is apparent that this function should be used instead of totalSupply() because the pool owes debt to the protocol in the form of unminted BPT, which should be taken into consideration. I leave here a link to a similar issue found a few weeks ago: https://github.com/sherlock-audit/2023-10-notional-judging/issues/36

2 submodules are affected BalancerPoolTokenPrice and AuraBalancerSupply.

Impact

Potentially incorrect prices returned by the Balancer feed, which could impact the operation of the RBS module. For instance, using the wrong price during a swap may lead to financial losses for the protocol.

Code Snippet

https://github.com/sherlock-audit/2023-11-olympus/blob/main/bophades/src/modules/PRICE/submodules/feeds/BalancerPoolTokenPrice.sol#L402

https://github.com/sherlock-audit/2023-11-olympus/blob/main/bophades/src/modules/SPPLY/submodules/AuraBalancerSupply.sol#L345

Tool used

Manual Review

Recommendation

Use getActualSupply() instead totalSupply() for all Balancer pools that support it.

Duplicate of #155

sherlock-audit / 2023-11-olympus-judging

ge6a - Using incorrect function to determine the token supply in a Balancer weighted pool #184