Lack of validation when calculating principalFilled can cause undesirable interactions

[code423n4]

Lines of code

https://github.com/code-423n4/2022-07-swivel/blob/main/Swivel/Swivel.sol#L127 https://github.com/code-423n4/2022-07-swivel/blob/main/Swivel/Swivel.sol#L227

Vulnerability details

Description

principalFilled = (a * o.principal) / o.premium; is called in 4 different functions but only 2 of these 4 functions are affected by the lack of this validation (the other 2 reverts). These 2 functions are initiateVaultFillingZcTokenInitiate and initiateVaultFillingVaultExit.

In the following scenarios, assume that 1) o.principal is less than o.premiums (bad order?) and 2) taker specifies a such that a * o.principal / o.premium rounds down to 0.

Do note that some money markets like Compound allows you to mint 0 which will return success i.e. deposit() can pass even with 0 passed in as an argument.

• initiateVaultFillingZcTokenInitiate: If principalFilled is 0, it means that the maker is not transferring any principal to Swivel despite getting paid premium a by the taker. As a result, no zc and vault positions are created (taker does not get his vault position despite paying for it). No fees will be paid to Swivel as well.
• initiateVaultFillingVaultExit: If principalFilled is 0, it means that the maker is not transferring any notional to the taker despite getting paid premium a by the taker. The zc position still resides with maker. No fees will be paid to Swivel as well.

Tools used

Manual analysis

Recommended Mitigation Steps

Require that principalFilled cannot be 0.

[JTraversa]

Same thing as #38, with a bit less relevance because it first requires the assumption that premium > principal (indicating a negative yield)

[bghughes]

Duplicate of #38