Change in `auctionMultiplier/auctionDecrement` change profitability of auctions and factory can steal all tokens from a basket abusing it

[code423n4]

Handle

0x0x0x

Vulnerability details

When factory changes auctionMultiplier or auctionDecrement profitability of bonded auctions change. There is no protection against this behaviour. Furthermore, factory owners can decide to get all tokens from baskets where they are bonded for the auction.

Proof of concept

1- Factory owners call bondForRebalance for an auction.

2- Factory owners sets auctionMultiplier as 0 and auctionDecrement as maximum value

3- settleAuction is called. newRatio = 0, since a = b = 0. All tokens can be withdrawn with this call, since tokensNeeded = 0.

Extra notes

Furthermore, even the factory owners does not try to scam users. In case auctionMultiplier or auctionDecrement is changed, all current auctionBonder from Auctions can only call settleAuction with different constraints. Because of different constraints, users/bonder will lose/gain funds.

Mitigation step

Save auctionDecrement and auctionMultiplier to global variables in Auction.sol, when startAuction is called.

[frank-beard]

Adding in protection from the global governance is definitely important.

[0xleastwood]

The warden has identified an issue whereby the factory owner can rug-pull baskets through the re-balancing mechanism. Because newRatio = 0, the basket improperly checks the tokens needed in the contract. However, this assumes that the factory owner is malicious which satisfies medium severity due to assets not being at direct risk.

The sponsor has decided to add additional protections (potentially via timelock) to mitigate this issue.

[0xleastwood]

This rug-pull is made even more difficult by the fact that newRatio must be >= minIbRatio. Because minIbRatio is behind timelock, I think this rug vector is unlikely or at least can only be used to steal a fixed amount of funds.