**BalancerOracle can be manipulated using read-only reentrancy**

[hats-bug-reporter[bot]]

Github username: @https://github.com/maarcweiss Submission hash (on-chain): 0x6bddb0c819575a51ced25c0fffb628214677f641f66c532e0c3b43e5a6b7d969 Severity: high severity

Description: BalancerOracle can be manipulated using read-only reentrancy

The function BalancerOracle.get_lp_price makes an external call to BalancerVault.getPoolTokens without checking the Balancer Vault's reentrancy guard. As a result, the oracle can be trivially manipulated to borrow at a different asset price than the actual one and to liquidate user positions prematurely.

https://github.com/VMEX-finance/vmex/blob/a2eafb4d3294b92eb88ab79732778c2dc2e4e06d/packages/contracts/contracts/protocol/lendingpool/LendingPool.sol#LL242C26-L242C26

https://github.com/VMEX-finance/vmex/blob/b0dc00c5dd6bdcac05827128d14dcdc730f19e1c/packages/contracts/contracts/protocol/lendingpool/LendingPoolCollateralManager.sol#L337-L340

In February, the Balancer team disclosed a read-only reentrancy vulnerability in the Balancer Vault. The detailed disclosure can be found here. In short, all Balancer pools are susceptible to manipulation of their external queries, and all integrations must now take an extra step of precaution when consuming data. Via reentrancy, an attacker can force token balances and BPT supply to be out of sync, creating very inaccurate BPT prices.

Some protocols, such as Sentiment, remained unaware of this issue for a few months and were later hacked as a result.

BalancerOracle.calc_balancer_lp_price makes a price calculation of the form f(balances) / pool.totalSupply() or f(balances) / pool.getActualSupply(), depending if legacy is true or false.  So it is clearly vulnerable to synchronization issues between the two data points.

https://github.com/VMEX-finance/vmex/blob/3ad2b901118f49b25eb709fc8a098b96a3dce1ea/packages/contracts/contracts/protocol/oracles/BalancerOracle.sol#L127-L147

PoC

A rough outline of the attack might look like this:

AttackerContract.flashLoan() ->
    // Borrow lots of tokens and trigger a callback.
    SomeProtocol.flashLoan() ->
        AttackerContract.exploit()

AttackerContract.exploit() ->
    // Join a Balancer Pool using the borrowed tokens and send some ETH along with the call.
    BalancerVault.joinPool() ->
        // The Vault will return the excess ETH to the sender, which will reenter this contract.
        // At this point in the execution, the BPT supply has been updated but the token balances have not.
        AttackerContract.receive()

AttackerContract.receive() ->
    // Liquidate a position using the same Balancer Pool as collateral.
    VMEXFinance.liquidationCall() ->
        // Call to the oracle to check the price.
        BalancerPairOracle.getPrice() ->
            // Query the token balances. At this point in the execution, these have not been updated (see above).
            // So, the balances are still the same as before the start of the large pool join.
            BalancerVaul.getPoolTokens()

            // Query the BPT supply. At this point in the execution, the supply has already been updated (see above).
            // So, it includes the latest large pool join, and as such the BPT supply has grown by a large amount.
            BalancerPool.getTotalSupply()

            // Now the price is computed using both balances and supply, and the result is much smaller than it should be.
            price = ((fairResA * pxA) + (fairResB * pxB))  / pool.totalSupply()

        // The position is liquidated under false pretenses.

SEVERITY

Critical. Fetching balancer pool positions allow attacker to perform read only reentrancy and borrow/liquidate due to unreliable price data.

A LINK TO THE GITHUB ISSUE

https://github.com/VMEX-finance/vmex/blob/a2eafb4d3294b92eb88ab79732778c2dc2e4e06d/packages/contracts/contracts/protocol/lendingpool/LendingPool.sol#LL242C26-L242C26

https://github.com/VMEX-finance/vmex/blob/b0dc00c5dd6bdcac05827128d14dcdc730f19e1c/packages/contracts/contracts/protocol/lendingpool/LendingPoolCollateralManager.sol#L337-L340

https://github.com/VMEX-finance/vmex/blob/3ad2b901118f49b25eb709fc8a098b96a3dce1ea/packages/contracts/contracts/protocol/oracles/BalancerOracle.sol#L127-L147

SOLUTION

The Balancer team recommends utilizing their official library to safeguard queries such as Vault.getPoolTokens.

[ksyao2002]

Please take look at line L38 in BalancerOracle.sol. We already perform a view re-entrancy guard based on best practices from Balancer.

VaultReentrancyLib.ensureNotInVaultContext(vault);