Platform: Code4rena
Start Date: 07/01/2022
Pot Size: $80,000 USDC
Total HM: 21
Participants: 37
Period: 7 days
Judge: 0xean
Total Solo HM: 14
Id: 71
League: ETH
Rank: 28/37
Findings: 3
Award: $87.54
🌟 Selected for report: 1
🚀 Solo Findings: 0
86.5379 INSURE - $30.29
52.5409 USDC - $52.54
egjlmn1
https://github.com/code-423n4/2022-01-insure/blob/main/contracts/PoolTemplate.sol#L390
In allocateCredit() an attacker can push into the indexList state variable
and in applyCover() and in resume() there is a loop that goes over all the indexes.
An attacker can't just add himself through calling allocateCredit() because there is check that he is listed in the registry.
What the attacker can do is create a lot of markets, and each time a market is created it will automatically call allocateCredit() and by doing this the attacker can increase the indexList as much as he wants.
If the array is too large, the for loop will iterate too many times for any gas amount, which will DOS those function calls. And just waste the gas limit for the users who call those functions
once a transaction reaches the gas limit, it will revert.
manual code review
have a cap on the indexList or only allow authorized market to join
#0 - oishun1112
2022-01-14T07:17:05Z
True, but creation of index pool is only allowed to the governance.
#1 - 0xean
2022-01-27T22:09:36Z
dupe of #352
egjlmn1
In most of the for loops you have, the condition of the for loop is i < array.length;
this a big waste of gas given that some of those arrays are state variables, meaning you access a storage variable in every single loop iteration.
A huge waste of gas for the state variable arrays and a smaller waste (but still a waste) for the calldata or memory. The amount of gas can pile up even for the calldata and memory depends on how big the array is.
Tested on remix
Manual code review
save the length of the array in a local variable and replace the array.length in the for loop's condition with this varaible
#0 - oishun1112
2022-01-13T14:45:23Z
2.9784 INSURE - $1.04
1.5637 USDC - $1.56
egjlmn1
in all of your for loops, you increase your loop variable using i++
it has 2 problems:
++i instead of i++)unchecked{}prefix arithmetic is a bit cheaper than postfix arithmetic, but if you do it in a for loop, this small amount of gas can pile up and be a big waste.
also, in solidity 0.8.0+, every arithmetic operation is checked for overflow and underflow, which adds a lot of gas to a single operation. Since in your for loop you don't have the risk for overflow, you can surround the operation in unchecked{} to save a lot of gas (which will save a huge amount since it saves a lot in a single loop iteration.)
Checked on remix
manual code review
change every i++ in your for loops to unchecked{++i}
2.4125 INSURE - $0.84
1.2666 USDC - $1.27
egjlmn1
In every single for function you have, you initialize the loop variable to 0, uint256 i = 0;
this is a waste of gas since the default value of uint256 is already 0.
This wastes 22 gas every time, which doesn't sound a lot but since you do the same mistake in every single for loop and the fix is really easy, you should fix it.
Tested on remix
Manual code review
remove the = 0 initialization of the loop variable.
#0 - oishun1112
2022-01-13T11:27:45Z