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: 25/37
Findings: 2
Award: $253.47
🌟 Selected for report: 4
🚀 Solo Findings: 0
37.3929 INSURE - $13.09
22.7028 USDC - $22.70
tqts
Bad accounting in withdrawalReq variable.
The withdrawalReq[msg.sender] value is not updated upon withdrawal, because request.amount -= _amount; in L230 updates a memory variable, not the storage.
Manual review
Replace L230 with withdrawalReq[msg.sender].amount -= _amount;
#0 - oishun1112
2022-01-13T11:58:56Z
https://github.com/code-423n4/2022-01-insure/blob/main/test/unitary/CDSTemplate/unitCDS.test.js#L1061 https://github.com/code-423n4/2022-01-insure/blob/main/test/unitary/CDSTemplate/unitCDS.test.js#L1124
you can see withdraw() decrease the requested amount successfully in the test code
#1 - oishun1112
2022-01-13T17:20:14Z
28.022 INSURE - $9.81
14.7115 USDC - $14.71
tqts
None
The for loop at L109-113 can be unrolled to remove the overhead of the loop itself, and avoid using an initialized-to-zero uint128 variable.
Manual review
Replace L109-113 with:
uint256 _allocation = (_shares[0] * _attributions) / MAGIC_SCALE_1E6; attributions[_beneficiaries[0]] += _allocation; _allocations[0] = _allocation; _allocation = (_shares[1] * _attributions) / MAGIC_SCALE_1E6; attributions[_beneficiaries[1]] += _allocation; _allocations[1] = _allocation;
28.022 INSURE - $9.81
14.7115 USDC - $14.71
tqts
None
The request.timestamp and parameters.getLockup(msg.sender) are used twice in the require statements, and both times summed.
Cache the sum value in a new variable. I've sent a similar report for IndexTemplate.withdraw() with a similar issue.
#0 - oishun1112
2022-01-13T10:56:34Z
#1 - oishun1112
2022-01-17T07:55:05Z
🌟 Selected for report: tqts
62.2711 INSURE - $21.79
32.6923 USDC - $32.69
tqts
None
The number of iterations in the second for loop, at L381-402 of IndexTemplate.sol can be reduced, and the zero-address check can be eliminated.
If the check in L363-366 succeeds, poolList[i].addr gets assigned address(0), and this is checked in the second for loop.
Manual review
In the else block at L372-377 a new index variable (let's call it skippedAmount, for the example) should be used for _poolList and incremented for each skipped withdraw. This, (along with removing L369), reduces effectively the number of loops in the next for loop, whose index should now go from 0 to skippedAmount, and doesn't need the if check of L382.
#0 - oishun1112
2022-02-02T10:19:55Z
lack of information
🌟 Selected for report: tqts
62.2711 INSURE - $21.79
32.6923 USDC - $32.69
tqts
None
The withdrawalReq[msg.sender].timestamp and parameters.getLockup(msg.sender) values are used twice in the require statements, and both times summed.
Manual review
Cache the sum value in a new variable. I've sent a similar report for IndexTemplate.withdraw() with a similar issue.
#0 - oishun1112
2022-01-13T10:56:28Z
2.9784 INSURE - $1.04
1.5637 USDC - $1.56
tqts
Default value of uint256 is 0, so there's no need to initialize the loop variables.
i++ consumes more gas than ++i.
The list of loops with these issues:
./Factory.sol:176: for (uint256 i = 0; i < _references.length; i++) { ./Factory.sol:186: for (uint256 i = 0; i < _conditions.length; i++) { ./IndexTemplate.sol:280: for (uint256 i = 0; i < _length; i++) { ./IndexTemplate.sol:348: for (uint256 i = 0; i < _length; i++) { ./IndexTemplate.sol:381: for (uint256 i = 0; i < _length; i++) { ./IndexTemplate.sol:462: for (uint256 i = 0; i < _poolLength; i++) { ./IndexTemplate.sol:655: for (uint256 i = 0; i < poolList.length; i++) { ./PoolTemplate.sol:343: for (uint256 i = 0; i < _ids.length; i++) { ./PoolTemplate.sol:671: for (uint256 i = 0; i < indexList.length; i++) { ./PoolTemplate.sol:703: for (uint256 i = 0; i < indexList.length; i++) {
Custom script
Replace i++ with ++i and remove the initialization.
#0 - oishun1112
2022-01-13T14:48:56Z
#1 - 0xean
2022-01-27T23:53:34Z
duplicate of #254
🌟 Selected for report: 0xngndev
Also found by: Dravee, Jujic, Meta0xNull, defsec, p4st13r4, robee, sirhashalot, tqts
2.9784 INSURE - $1.04
1.5637 USDC - $1.56
tqts
Higher gas consumption in deployment transaction and when the require conditions are met.
The list of long strings in scope contracts:
Require reason string length 36 -> ./CDSTemplate.sol:98 Require reason string length 35 -> ./CDSTemplate.sol:212 Require reason string length 34 -> ./CDSTemplate.sol:219 Require reason string length 44 -> ./Factory.sol:74 Require reason string length 44 -> ./IndexTemplate.sol:101 Require reason string length 36 -> ./IndexTemplate.sol:131 Require reason string length 35 -> ./IndexTemplate.sol:222 Require reason string length 34 -> ./IndexTemplate.sol:227 Require reason string length 38 -> ./IndexTemplate.sol:233 Require reason string length 36 -> ./IndexTemplate.sol:426 Require reason string length 40 -> ./InsureDAOERC20.sol:160 Require reason string length 37 -> ./InsureDAOERC20.sol:215 Require reason string length 37 -> ./InsureDAOERC20.sol:244 Require reason string length 35 -> ./InsureDAOERC20.sol:245 Require reason string length 38 -> ./InsureDAOERC20.sol:250 Require reason string length 33 -> ./InsureDAOERC20.sol:297 Require reason string length 34 -> ./InsureDAOERC20.sol:302 Require reason string length 36 -> ./InsureDAOERC20.sol:330 Require reason string length 34 -> ./InsureDAOERC20.sol:331 Require reason string length 44 -> ./Ownership.sol:37 Require reason string length 44 -> ./Ownership.sol:45 Require reason string length 44 -> ./Parameters.sol:50 Require reason string length 44 -> ./PoolTemplate.sol:149 Require reason string length 36 -> ./PoolTemplate.sol:183 Require reason string length 35 -> ./PoolTemplate.sol:310 Require reason string length 34 -> ./PoolTemplate.sol:317 Require reason string length 38 -> ./PoolTemplate.sol:322 Require reason string length 37 -> ./PoolTemplate.sol:382 Require reason string length 37 -> ./PoolTemplate.sol:423 Require reason string length 40 -> ./PoolTemplate.sol:476 Require reason string length 40 -> ./PoolTemplate.sol:608 Require reason string length 44 -> ./PremiumModels/BondingPremium.sol:33 Require reason string length 38 -> ./PremiumModels/BondingPremium.sol:65 Require reason string length 44 -> ./Registry.sol:23 Require reason string length 44 -> ./Vault.sol:45 Require reason string length 34 -> ./Vault.sol:153 Require reason string length 34 -> ./Vault.sol:186 Require reason string length 40 -> ./Vault.sol:300 Require reason string length 40 -> ./Vault.sol:329
Custom script
Shorten the revert reason strings to fit in 32 bytes.
#0 - oishun1112
2022-01-13T17:04:08Z
🌟 Selected for report: tqts
62.2711 INSURE - $21.79
32.6923 USDC - $32.69
tqts
None
In L197 of IndexTemplate, a _balance variable is created and initialized to the balance of msg.sender. However that variable is used only once in the function.
Manual review
Replace L198 with require(balanceOf(msg.sender) >= _amount, "ERROR: REQUEST_EXCEED_BALANCE"); and remove L197