Platform: Code4rena
Start Date: 18/04/2024
Pot Size: $36,500 USDC
Total HM: 19
Participants: 183
Period: 7 days
Judge: Koolex
Id: 367
League: ETH
Rank: 62/183
Findings: 2
Award: $129.79
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: Circolors
Also found by: 0xtankr, AamirMK, Al-Qa-qa, FastChecker, Infect3d, SBSecurity, Strausses, T1MOH, VAD37, carrotsmuggler, gumgumzum
122.4433 USDC - $122.44
https://github.com/code-423n4/2024-04-dyad/blob/main/src/core/Vault.kerosine.bounded.sol#L23-L30
As mentioned in the Readme.md, that the deploy script is the whole migration. Not including the setUnboundedKerosineVault function could lead to unexpected failures. As the asset_price() function for the bounded Kerosine vault will throw error while called in multiple places in the VaultManagerV2 contract
https://github.com/code-423n4/2024-04-dyad/blob/main/src/core/Vault.kerosine.bounded.sol#L49
https://github.com/code-423n4/2024-04-dyad/blob/main/src/core/VaultManagerV2.sol#L146
https://github.com/code-423n4/2024-04-dyad/blob/main/src/core/VaultManagerV2.sol#L197
Manual Review
Include the function setUnboundedKerosineVault () to set the address of the newly deployed unboundedKerosineVault contract at this line https://github.com/code-423n4/2024-04-dyad/blob/main/script/deploy/Deploy.V2.s.sol#L89 before transferring Ownership to the multi-sign address
Other
#0 - c4-pre-sort
2024-04-28T19:06:43Z
JustDravee marked the issue as duplicate of #829
#1 - c4-pre-sort
2024-04-29T09:22:54Z
JustDravee marked the issue as sufficient quality report
#2 - c4-judge
2024-05-05T10:52:11Z
koolexcrypto marked the issue as unsatisfactory: Invalid
#3 - c4-judge
2024-05-29T12:33:43Z
koolexcrypto marked the issue as satisfactory
🌟 Selected for report: TheSavageTeddy
Also found by: 0x175, 0x486776, 0xnev, AamirMK, AlexCzm, ArmedGoose, BiasedMerc, CaeraDenoir, Egis_Security, Jorgect, KYP, MrPotatoMagic, PoeAudits, SBSecurity, SovaSlava, VAD37, adam-idarrha, alix40, carrotsmuggler, d_tony7470, dimulski, grearlake, josephdara, ljj, n0kto, okolicodes, sashik_eth, sil3th, turvy_fuzz
7.3512 USDC - $7.35
https://github.com/code-423n4/2024-04-dyad/blob/main/src/core/VaultManager.sol#L70
The remove() function in the VaultManagerV2 checks for the asset in the particular vault to be not greater than zero, before removing the vault. This can be misused by a malicious actor to front run this transaction with a call to the deposit() function to the same id and vault with a small minimum amount to block the user from removing the vault. This could have higher impact if the user desires to remove one vault and add another one, as the number of vaults is restricted.
https://github.com/code-423n4/2024-04-dyad/blob/main/src/core/VaultManager.sol#L63-L73
deposit function can be called by anyone https://github.com/code-423n4/2024-04-dyad/blob/main/src/core/VaultManager.sol#L75-L86
Manual Review
1.deposit function can be modified to accept atleast a minimum value 2. remove function can send the excess assets if any, back to the dNFT onwer while the vault is removed, instead of checking for the value to be zero
Invalid Validation
#0 - c4-pre-sort
2024-04-27T13:33:32Z
JustDravee marked the issue as duplicate of #489
#1 - c4-pre-sort
2024-04-29T09:25:47Z
JustDravee marked the issue as sufficient quality report
#2 - c4-judge
2024-05-05T20:20:43Z
koolexcrypto marked the issue as not a duplicate
#3 - c4-judge
2024-05-06T08:57:23Z
koolexcrypto marked the issue as duplicate of #118
#4 - c4-judge
2024-05-11T12:24:28Z
koolexcrypto marked the issue as satisfactory