InsureDAO contest - hyh's results

Handle

hyh

Vulnerability details

Impact

If market creation wasn’t successful the Factory.createMarket will fail with low level message on trying to use a freshly created market contract. This behavior can be an issue for troubleshooting and further programmatic usages by other projects.

Proof of Concept

createMarket uses _createClone to instantiate new contract:

https://github.com/code-423n4/2022-01-insure/blob/main/contracts/Factory.sol#L210

createClone doesn't check the result of create opcode, returning any result:

https://github.com/code-423n4/2022-01-insure/blob/main/contracts/Factory.sol#L256

createMarket uses the market created, and when creation has failed the system will revert in Registry function down the line:

https://github.com/code-423n4/2022-01-insure/blob/main/contracts/Registry.sol#L55

Recommended Mitigation Steps

Check for zero address create output and fail with the corresponding message

Handle

hyh

Vulnerability details

Impact

System will fail on low-level subtraction without proper logic level error, which can be an issue for troubleshooting and further programmatic usages by other projects.

Proof of Concept

Whenever user lacks _attributions (Vault shares) for the withdraw amount requested, the system will fail on subtraction:

https://github.com/code-423n4/2022-01-insure/blob/main/contracts/Vault.sol#L160

Recommended Mitigation Steps

Consider adding a check for the enough _attributions throwing a corresponding error

Handle

hyh

Vulnerability details

Impact

System will fail with obscure low-level array access message without proper logic level error, which can be an issue for troubleshooting and further programmatic usages by other projects.

Proof of Concept

Whenever _references or _conditions are too big the system will fail on index out of bounds error:

https://github.com/code-423n4/2022-01-insure/blob/main/contracts/Factory.sol#L175-191

Recommended Mitigation Steps

Consider adding a check for the upper bound of arrays length

Handle

hyh

Vulnerability details

Impact

Assert will consume all the available gas, providing no additional benefits when being used instead of require, that both returns gas and allows for error message. This way using assert in production isn't recommended.

Proof of Concept

Two contracts now use assert:

InsureDAOERC20

https://github.com/code-423n4/2022-01-insure/blob/main/contracts/InsureDAOERC20.sol#L32

Vault

https://github.com/code-423n4/2022-01-insure/blob/main/contracts/Vault.sol#L168

Recommended Mitigation Steps

Substitute assert with require in both cases

Handle

hyh

Vulnerability details

Impact

setKeeper / utilize descriptions state that it is controller who is set / can run utilize, while keeper and controller are two separate roles, which don't have to coincide.

I.e. the descriptions now mix up the roles and are confusing this way.

Proof of Concept

setKeeper:

https://github.com/code-423n4/2022-01-insure/blob/main/contracts/Vault.sol#L499

utilize:

https://github.com/code-423n4/2022-01-insure/blob/main/contracts/Vault.sol#L339

Recommended Mitigation Steps

Update the descriptions to relate to the keeper role.

Handle

hyh

Vulnerability details

Impact

Gas is overspent on the function call

Proof of Concept

availableBalance calls totalLiquidity() twice:

https://github.com/code-423n4/2022-01-insure/blob/main/contracts/PoolTemplate.sol#L835

Recommended Mitigation Steps

Save the call result to memory and use it

Handle

hyh

Vulnerability details

Impact

Gas is overspend on checks

Proof of Concept

Amounts are subtracted after require checks in two cases, so the subtractions itself can be unchecked.

withdraw: check https://github.com/code-423n4/2022-01-insure/blob/main/contracts/PoolTemplate.sol#L318 subtraction https://github.com/code-423n4/2022-01-insure/blob/main/contracts/PoolTemplate.sol#L327

withdrawCredit: check https://github.com/code-423n4/2022-01-insure/blob/main/contracts/PoolTemplate.sol#L425 subtraction https://github.com/code-423n4/2022-01-insure/blob/main/contracts/PoolTemplate.sol#L439

Recommended Mitigation Steps

Make unchecked both subtraction operations

Handle

hyh

Vulnerability details

Impact

Underlying and index tokens are mixed up in the worth() function description, making code and its description conflicting

Proof of Concept

Worth() computes how many iTokens correspond to given amount of underlying. The description says otherwise, mixing them up:

https://github.com/code-423n4/2022-01-insure/blob/main/contracts/PoolTemplate.sol#L794-798

Recommended Mitigation Steps

Fix the description to say that ‘_value' is the amount of underlying, while the '_amount' is the corresponding output quantity of iTokens