Creator of pie can mint any amount of _initialSupply, and drain underlying tokens via exitPool

[code423n4]

Handle

hubble

Vulnerability details

Impact

The Creator of pie or msg.sender of bakePie in PieFactoryContract, can set any high value of _initialSupply and get the ERC20 tokens minted. There is no relation to the _intialSupply and the amount of underlying Tokens added to the Pie during the bakePie function call. Because of this the msg.sender of bakePie, can exitPool and drain a huge underlying tokens later on.

As compared to this when others use joinPool, the amount of ERC20 tokens minted and the amount of underlying tokens transferred into the pool are proportionate.

Proof of Concept

File : PieFactoryContract.sol Function : bakePie Line : 84 thru 101

    // Init erc20 facet
    pie.initialize(_initialSupply, _name, _symbol);

    // Transfer and add tokens
    for (uint256 i = 0; i < _tokens.length; i++) {
        IERC20 token = IERC20(_tokens[i]);
        token.safeTransferFrom(msg.sender, address(pie), _amounts[i]);
        pie.addToken(_tokens[i]);
    }

    // Unlock pool
    pie.setLock(1);

    // Uncap pool
    pie.setCap(uint256(-1));

    // Send minted pie to msg.sender
    pie.transfer(msg.sender, _initialSupply);

Tools Used

Manual review

Recommended Mitigation Steps

The function bakePie should not have _initialSupply parameter. The ERC20 minted should be proportional to the _amounts of underlying _tokens supplied, similar to the function joinPool in BasketFacet.sol For example, Line 101 : pie.transfer(msg.sender, _amount);

Line 85 : pie.initialize(_initialSupply, _name, _symbol); this can be initialised with some low value, and the amount be tranferred to the Contract Owner.

[loki-sama]

This is the intended initialization process. Before a pie is baked there is no underlying token and this creates the ratio of underlying to basket token.

[0xleastwood]

I think this makes sense. The account baking a pie sends its own tokens so should have ownership over the underlying tokens. Marking as invalid.