`newIbRatio` update math depends on how often it's called

[code423n4]

Handle

cmichel

Vulnerability details

As I understand it, the handleFees should rebalance the ibRatio such that the licenseFee is taken on all deposits. I.e., a yearly licenseFee of 12% (0.12e18) should result in an ibRatio of 1.0 - 0.12 = 0.88 after one year. Because with this value, burning the initial 1e18 basket tokens will call pushUnderlying(1e18) and pay out 88% of the initial token deposit.

This is correct when calling handleFees once per year, but when it's called every week/month it will not be exact. This is similar to simple interest vs compound interest. The individual updates compound wrong.

For example, assume handleFees is called once every 6 months (feePct = 6/12 * 12% = 0.06e18).

# assume startSupply of 1e18 (see factory), and initial ibRatio of 1e18 (BASE)
# and no other deposits in between
licenseFee = 12% = 0.12e18;
# mints "fee" in total, split into publisher and factory

# call it after 6 months once
=> startSupply = 1e18
fee = startSupply * 0.06 / (1.0 - 0.06) = 1.0 * 0.063829787
newIbRatio = 1.0 * 1.0 / (1.063829787) = 0.94
=> new startSupply = 1.063829787e18

# call it after another 6 months
fee = startSupply * 0.06 / (1.0 - 0.06) = 1.063829787 * 0.063829787 = 0.067904029
newIbRatio = ibRatio * 1.063829787 / (1.063829787 + 0.067904029)
            = 0.94 * 1.063829787 / (1.063829787 + 0.067904029)
            = 0.8836 > 0.88

Impact

The fees are underestimated.

Recommended Mitigation Steps

It's probably best to just leave it like this as a compounding formula would be too complex and gas-intensive.

[GalloDaSballo]

Finding is valid, would recommend updating comments to reflect this property and consider if it's possible to refactor the code to properly account for time expired