Platform: Code4rena
Start Date: 07/08/2023
Pot Size: $36,500 USDC
Total HM: 11
Participants: 125
Period: 3 days
Judge: alcueca
Total Solo HM: 4
Id: 274
League: ETH
Rank: 66/125
Findings: 1
Award: $9.82
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: RED-LOTUS-REACH
Also found by: 0x3b, 0x4non, 0xCiphky, 0xDING99YA, 0xDetermination, 0xE1, 0xG0P1, 0xStalin, 0xWaitress, 0xbrett8571, 0xhacksmithh, 0xkazim, 0xmuxyz, 0xweb3boy, 14si2o_Flint, AlexCzm, Alhakista, Bube, Bughunter101, Deekshith99, Eeyore, Giorgio, HChang26, InAllHonesty, JP_Courses, KmanOfficial, MatricksDeCoder, Mike_Bello90, MrPotatoMagic, Naubit, QiuhaoLi, RHaO-sec, Raihan, Rolezn, SUPERMAN_I4G, Shubham, Silverskrrrt, Strausses, T1MOH, Topmark, Tripathi, Watermelon, _eperezok, aakansha, auditsea, audityourcontracts, ayden, carlos__alegre, castle_chain, cducrest, ch0bu, d23e, deadrxsezzz, deth, devival, erebus, fatherOfBlocks, halden, hassan-truscova, hpsb, hunter_w3b, imkapadia, immeas, jat, kaden, kaveyjoe, klau5, koxuan, kutugu, ladboy233, lanrebayode77, leasowillow, lsaudit, markus_ether, matrix_0wl, merlin, nemveer, ni8mare, nonseodion, oakcobalt, owadez, p_crypt0, pipidu83, piyushshukla, popular00, ppetrov, rjs, sandy, sl1, supervrijdag, tay054, thekmj, wahedtalash77, windhustler, zhaojie
9.8204 USDC - $9.82
Protocol implements a mitigation measurement to reduce internal damage in case of a lending market vulnerability, but it doesn't work for ongoing epoch.
Governance can call GaugeController::change_gauge_weight() in emergency situations and temporarily set the weight to 0. The implementation allows the adjustment of weight exclusively for the subsequent epoch:
uint256 next_time = ((block.timestamp + WEEK) / WEEK) * WEEK;
Temporary setting the current epoch weight to 0 is not possible. Therefore, if a lending market vulnerability arises, it could result in leakage of rewards.
function _change_gauge_weight(address _gauge, uint256 _weight) internal { uint256 old_gauge_weight = _get_weight(_gauge); uint256 old_sum = _get_sum(); uint256 next_time = ((block.timestamp + WEEK) / WEEK) * WEEK; points_weight[_gauge][next_time].bias = _weight; time_weight[_gauge] = next_time; uint256 new_sum = old_sum + _weight - old_gauge_weight; points_sum[next_time].bias = new_sum; time_sum = next_time; }
Manual review.
Reconsider and allow current epoch weight to be updated by governance: Replace
uint256 next_time = ((block.timestamp + WEEK) / WEEK) * WEEK;
with
uint256 next_time = ((block.timestamp) / WEEK) * WEEK; ## Assessed type Governance
#0 - 141345
2023-08-13T06:46:22Z
QA might be more appropriate.
#1 - c4-sponsor
2023-08-16T15:01:09Z
OpenCoreCH marked the issue as sponsor acknowledged
#2 - OpenCoreCH
2023-08-16T15:03:24Z
This was done on purpose such that governance cannot remove already accrued rewards. Of course, this is a tradeoff between centralization and response duration in case of emergencies, but the current implementation is relatively balanced there in my opinion, as users can be sure that no accrued rewards will be stolen and the response duration is still pretty good (worst case is that the rewards of up to 1 epoch for one market are lost in case of an exploit, so quite a small impact).
#3 - c4-judge
2023-08-24T21:24:43Z
alcueca marked the issue as satisfactory
#4 - JeffCX
2023-08-28T20:29:54Z
while I respect judge's expertise
This was done on purpose such that governance cannot remove already accrued rewards. Of course, this is a tradeoff between centralization and response duration in case of emergencies, but the current implementation is relatively balanced there in my opinion, as users can be sure that no accrued rewards will be stolen and the response duration is still pretty good (worst case is that the rewards of up to 1 epoch for one market are lost in case of an exploit, so quite a small impact).
based on this comments, the current implementation seems like a design choice but not a vulnerability, so I politely think the severity is QA
#5 - c4-judge
2023-08-28T21:25:00Z
alcueca changed the severity to QA (Quality Assurance)
#6 - c4-judge
2023-08-28T21:25:09Z
alcueca marked the issue as grade-a
#7 - alcueca
2023-08-28T21:26:41Z
I'm going to agree with @JeffCX, given that the alternative in which governance can set current epoch weight to zero also results in a loss of assets makes it clear that it is a design decision that should just be documented.