Kuiper contest - kenzo's results

Handle

kenzo

Vulnerability details

handleFees does not update lastFee if startSupply == 0. This means that wrongly, extra fee tokens would be minted once the basket is resupplied and handleFees is called again.

Impact

Loss of user funds. The extra minting of fee tokens comes on the expense of the regular basket token owners, which upon withdrawal would get less underlying than their true share, due to the dilution of their tokens' value.

Proof of Concept

Scenario:

• All basket token holders are burning their tokens. The last burn would set totalSupply to 0.
• After 1 day, somebody mints basket tokens. handleFees would be called upon mint, and would just return since totalSupply == 0. Note: It does not update lastFee.

} else if (startSupply == 0) {
            return;

https://github.com/code-423n4/2021-12-defiprotocol/blob/main/contracts/contracts/Basket.sol#L136:#L137

• The next block, somebody else mints a token. Now handleFees will be called and will calculate the fees according to the current supply and the time diff between now and lastFee:

uint256 timeDiff = (block.timestamp - lastFee);

https://github.com/code-423n4/2021-12-defiprotocol/blob/main/contracts/contracts/Basket.sol#L139 But as we saw, lastFee wasn't updated in the previous step. lastFee is still the time of 1 day before - when the last person burned his tokens and the basket supply was 0. So now the basket will mint fees as if a whole day has passed since the last calculation, but actually it only needs to calculate the fees for the last block, since only then we had tokens in the basket.

Recommended Mitigation Steps

Set lastFee = block.timestamp if startSupply == 0.

Handle

kenzo

Vulnerability details

In fees calculation, division is being used in the midst of the calculation, not at the end of it. This leads to lost precision in fee amount (as solidity doesn't save remainder of division). Division should happen at the end to maintain precision.

Impact

Lost fees. The exact amount depends on the parameters set and being tested. According to a few tests I ran, it seems that in normal usage, 1% of fees are lost. In some cases even 7.5% of fees.

Proof of Concept

Division in the midst of a calculation:

uint256 feePct = timeDiff * licenseFee / ONE_YEAR;
uint256 fee = startSupply * feePct / (BASE - feePct);

_mint(publisher, fee * (BASE - factory.ownerSplit()) / BASE);
_mint(Ownable(address(factory)).owner(), fee * factory.ownerSplit() / BASE);

https://github.com/code-423n4/2021-12-defiprotocol/blob/main/contracts/contracts/Basket.sol#L140:#L145 It's a little hard to share a POC script as it involves changing the .sol file so I tested it manually. But after moving the division to the end using the mitigation below, I saw 1%-7% increases in fees minted. Usually 1%.

Recommended Mitigation Steps

We want to firstly do all multiplication and lastly do all the division. So remove the usage of feePct and instead set fee to be:

uint256 fee = startSupply * licenseFee * timeDiff / ONE_YEAR / (BASE - licenseFee);