Platform: Code4rena
Start Date: 28/09/2023
Pot Size: $36,500 USDC
Total HM: 5
Participants: 115
Period: 6 days
Judge: 0xDjango
Total Solo HM: 1
Id: 290
League: ETH
Rank: 84/115
Findings: 1
Award: $4.37
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: Bauchibred
Also found by: 0x3b, 0xDetermination, 0xMosh, 0xScourgedev, 0xTheC0der, 0xTiwa, 0xWaitress, 0xdice91, 0xfusion, 0xpiken, 0xprinc, 0xweb3boy, ArmedGoose, Aymen0909, Breeje, Brenzee, Daniel526, DavidGiladi, DeFiHackLabs, Flora, Fulum, HChang26, Hama, IceBear, J4X, Krace, KrisApostolov, Maroutis, Mirror, MohammedRizwan, Norah, PwnStars, SPYBOY, TangYuanShen, Testerbot, ThreeSigma, Tricko, al88nsk, alexweb3, ast3ros, berlin-101, bin2chen, blutorque, btk, d3e4, deth, e0d1n, ether_sky, ge6a, gkrastenov, glcanvas, hals, imare, inzinko, jkoppel, jnforja, joaovwfreire, josephdara, kutugu, lotux, lsaudit, mahdirostami, merlin, n1punp, nadin, neumo, nisedo, nobody2018, oakcobalt, orion, peanuts, pep7siup, pina, ptsanev, rokinot, rvierdiiev, said, santipu_, sashik_eth, seerether, squeaky_cactus, terrancrypt, tonisives, twicek, vagrant, xAriextz, y4y
4.3669 USDC - $4.37
Markets cannot be removed. Any mistake or problem with the VToken market or its address when added, or any future problem arising with the market, will essentially brick the Prime contract.
A market is added using addMarket() which pushes it into the list of allMarkets. There is no way to remove a market from this list.
Several functions invoke allMarkets in a loop. Thus if any market starts misbehaving, or was mistakenly added, or otherwise becomes undesirable, this has a permanent effect on most of Prime which cannot be corrected. The potential points of failure would be numerous, but difficult to determine. It could be that the oracle loses support or in other ways cannot appropriately manage the price anymore, such as due to market volatility, or that the underlying token loses its appeal or even becomes compromised. Perhaps the greatest danger is the possibility of adding an incorrect market address, or adding a misconfigured VToken.
Furthermore, Prime is initialized with a max loops limit. This limit restricts how many markets can be added. The function to change this limit is internal and only called in the initializer. The loop limit thus cannot be changed. So not only can one not replace a market, one also cannot let the old market be and just add the new one on top. So even if one doesn't necessarily want to remove markets, some markets may become obsolete or of marginal utility. Then one cannot keep up with the ever evolving market landscape and update the market portfolio.
Add functionality for removing markets. Possibly also a way for the owner to _setMaxLoopsLimit().
DoS
#0 - 0xRobocop
2023-10-04T21:51:23Z
Seems like a decision decision.
Marking as primary for sponsor review anyways.
#1 - c4-pre-sort
2023-10-04T21:51:29Z
0xRobocop marked the issue as primary issue
#2 - c4-pre-sort
2023-10-04T21:51:34Z
0xRobocop marked the issue as high quality report
#3 - c4-sponsor
2023-10-24T16:18:07Z
chechu marked the issue as disagree with severity
#4 - chechu
2023-10-24T16:19:06Z
Regarding the missing setMaxLoopsLimit, it’s a fair point. We would consider it QA.
Regarding removing a market, we don't think it's valid.
Removing a market is not a simple scenario. There could be users with claimable interests, so we should wait them for to claim their rewards.
There are available alternatives to “disable” a market. For example, we could set the multipliers of the market, used to calculate the user scores, to zero. This way no more rewards will be allocated to the users.
If the issue is in the market contract, Venus VToken contracts are upgradable, so we could resolve the potential issue with an upgrade (it would depend on the specific problem).
#5 - c4-sponsor
2023-10-24T18:45:49Z
chechu (sponsor) disputed
#6 - c4-judge
2023-10-31T19:00:56Z
fatherGoose1 changed the severity to QA (Quality Assurance)
#7 - fatherGoose1
2023-10-31T19:01:42Z
Agree with sponsor. There are various ways to effectively set a market as inactive. The recommendation to add an external function to set the max loop limit is valid.