`Basket.sol#auctionBurn` calculates `ibRatio` wrong

[code423n4]

Handle

0x0x0x

Vulnerability details

The function is implemented as follows:

function auctionBurn(uint256 amount) onlyAuction nonReentrant external override {
        uint256 startSupply = totalSupply();
        handleFees(startSupply);
        _burn(msg.sender, amount);

        uint256 newIbRatio = ibRatio * startSupply / (startSupply - amount);
        ibRatio = newIbRatio;

        emit NewIBRatio(newIbRatio);
        emit Burned(msg.sender, amount);
    }

When handleFees is called, totalSupply and ibRatio changes accordingly, but for newIbRatio calculation tokens minted in handleFees is not included. Therefore, ibRatio is calculated higher than it should be. This is dangerous, since last withdrawing user(s) lose their funds with this operation. In case this miscalculation happens more than once, newIbRatio will increase the miscalculation even faster and can result in serious amount of funds missing. At each time auctionBurn is called, at least 1 day (auction duration) of fees result in this miscalculation. Furthermore, all critical logic of this contract is based on ibRatio, this behaviour can create serious miscalculations.

Mitigation step

Rather than

uint256 newIbRatio = ibRatio * startSupply / (startSupply - amount);

A practical solution to this problem is calculating newIbRatio as follows:

uint256 supply = totalSupply();
uint256 newIbRatio = ibRatio * (supply + amount) / supply;

[0xleastwood]

The warden has identified an issue whereby newIbRatio uses an incorrect startSupply variable which is under-represented. As new tokens may be minted in handleFees(), this will lead to an incorrect ibRatio which is used in all other areas of the protocol. A lower ibRatio causes pushUnderlying() and pullUnderlying() to be improperly accounted for. As a result, burning basket tokens will redeem a smaller amount of underlying tokens and minting basket tokens will require a smaller amount of underlying tokens.

This causes the protocol to leak value from all basket token holders but it does not allow assets to be stolen. As such, I think this is better put as a medium severity issue.